Reorder select's operands to put the condition last.

[sunfishcode]

As discussed in #469, this PR proposes reordering select's operands to put the condition last. This allows it to evaluate the condition last under the common evaluation rules, which is expected to enable more optimization opportunities.

[titzer]

Do you also want to mention something for br_if that yields a value? Same problem.

I actually already have a v8-native branch that implements this.

[titzer]

By "something" above, I mean, that the condition comes last (after the value) for br_if.

[sunfishcode]

I'm happy to submit a PR to the spec repo to reorder the inputs to br_if (that's how I originally implemented it in LLVM too), but I don't think the design repo cares, because the label in a br_if is an immediate field, rather than an operand.

[titzer]

I am referring to the value in a br_if $label value cond.

[sunfishcode]

Ah ok. Some parts of AstSemantics.md say that br and br_if don't have return values so I was confused. I've updated this now, and a few related things.

[kripken]

it is instead of it's feels better to me given the overall formality here

[sunfishcode]

Changed "it's" to "it is".

[jcbeyler]

Now I see this conversation :)... Had I known I would not have created WebAssembly/spec#200...

[jcbeyler]

LGTM btw

[rossberg]

Should we really do this for br_if? The other operand is optional, and having an optional argument not at the end is going to be rather ugly for any sort of text format. (Worse, with a future extension to multiple values it may even become a list.)

[titzer]

In v8-native, one can just use a nop for the value.

As for multiple valuels: If we really have br_if and br with multiple
return values in the future, it's probably going to require the number of
values to be an immediate. Otherwise, to determine how many values
subexpressions, one would have to look at the block to which the br
branches. I can see that being not so reliable, because it may not even be
clear from the context how many values the block is expected to produce,
either.

On Thu, Dec 3, 2015 at 10:08 AM, rossberg-chromium <notifications@github.com

wrote:

Should we really do this for br_if? The other operand is optional, and
having an optional argument not at the end is going to be rather ugly for
any sort of text format. (Worse, with a future extension to multiple values
it may even become a list.)

—
Reply to this email directly or view it on GitHub
#489 (comment).

[ghost]

Is there a good reason to have the result of br_if' unconditionally evaluated? It seems fine to me for(br_if ?) ;; = (if_else (br ?) (nop))` Then the optional result expression is naturally last?

If supporting multiple values then the expressions will need to support this so there will still be only one result expression but it will have a multiple value type. An operator will be needed to build multiple values from multiple single value expressions. E.g. (br_if <cond> <var> (values <exp1> <exp2> ...))

[kg]

Should we really do this for br_if? The other operand is optional, and having an optional argument not at the end is going to be rather ugly for any sort of text format.

Generally speaking, having variable argument counts to opcodes is a big complication and we should constrain it to as few opcodes as possible. If there are two argument-counts possible for something like br_if, they should be separate opcodes, just like the if/if_else split. This is important for file size, validation robustness, and decoder complexity.

[rossberg]

@titzer, my remark was more about the concrete syntax of the hypothetical textual format, which I assume would want to actually make the operand optional, while also representing the order of evaluation correctly. In practice, I think the eval order will hardly ever matter, so we should perhaps pick what reads better in text?

(As for the potential case of multiple args, there is no more need to make the count explicit (in textual syntax) than for, say, function calls. Typing would be a trivial generalisation of what we already have now, see the original prototype.)

@kg, separate opcodes would not be very natural in the light of an extension to multi values, where br or return would naturally become variadic anyway. In contrast, if is a different beast.

[kg]

@rossberg-chromium
Even with multi-values there would still be two forms of br and return, the no-value form and the value form. The 'value' in that case would merely be a tuple instead of a single argument. Or are you saying that the # of values in those cases would still be variadic so it would be legal for a function to decide to return 1 value, 3 values, or 5 values? That sort of variance does not match the fully static approach we have to types and type signatures right now. Even if we want to support a single function containing 1-arg, 3-arg and 5-arg returns, that still doesn't justify making every instance of return or br in an application carry around an argument count. It's bad design.

Furthermore, we have yet to actually prove we need multiple return values or demonstrate that we can do them without significantly overhauling wasm. Compromising on file size, decode simplicity and validation simplicity right now for a questionable future feature is not something I can buy without strong justification.

[rossberg]

@kg, ah, no, arity would still be fully static, of course. As for the empty case, with multivalues it would not be special at all, it's just the N=0 case. You could make that a special opcode, if you're so inclined, but that wouldn't be any more natural than making, say, a function call with 0 arguments a special opcode. It might not even be more frequent.

But all that is a bit of a tangent. My main point here is not to paint ourselves into a syntactic corner too much unless we have to.

[ghost]

Could someone clarify why you would want to unconditionally evaluate the result expression for br_if?

I understand why you would do this for select but the result expression for if is not evaluated before the condition and not unconditionally.

[titzer]

I think the reason for the unconditional evaluation is to avoid having two
branches. For example, if the result expression has a side effect, then the
not-taken case has to branch around it in order to not evaluate the side
effect. Then the generated code looks like:

(eval cond)
jz not_taken:
(eval value)
jump dst
not_taken:
...

On Thu, Dec 3, 2015 at 1:47 PM, JSStats notifications@github.com wrote:

Could someone clarify why you would want to unconditionally evaluate the
result expression for br_if?

I understand why you would do this for select but the result expression
for if is not evaluated before the condition and not unconditionally.

—
Reply to this email directly or view it on GitHub
#489 (comment).

[ghost]

@titzer Ok, thanks. So if people actually want conditional evaluation they can use (if <cond> (br <> <expr>)) and if unconditional evaluation is ok they use br_if and avoid firstly writing the value to a local variable. Sounds worthy of a note in the rationale.

[sunfishcode]

@rossberg-chromium What we specifically want is for br_if's "result value" operand to be evaluated first, before its "condition" operand. Changing the operand order is a simple way to do this. The other obvious way would be to say that br_if has a custom evaluation order, which seems less nice.

@kg I am hoping to experiment with multiple return values from statements, and to use that to approximate the potential for get_local/set_local reduction (which would be the main advantage). What happens next will depend on what we learn.

@JSStats Yes, as @titzer says, making the return value unconditional avoids the need for two branches. Another way of looking at it is: br_if ends one basic block and transfers control to one of two others; the "return value" of br_if can be used for any value which is defined near the end of the first block and used near the beginning in the others. We don't yet have data on how often this liveness pattern happens, but this is the situation for the rest of the statements=expressions design too, so our first task is to collect data, and we can adjust the design accordingly.

[rossberg]

@sunfishcode, I understand that both operands should always be evaluated, and I agree that a custom evaluation order would be ugly. But why does it matter that the condition operand is evaluated last? What's the use case?

[titzer]

@rossberg-chromium: read up. It's because two branches are necessary.

On Thu, Dec 3, 2015 at 5:14 PM, rossberg-chromium notifications@github.com
wrote:

@sunfishcode https://github.com/sunfishcode, I understand that both
operands should always be evaluated, and I agree that a custom evaluation
order would be ugly. But why does it matter that the condition operand is
evaluated last? What's the use case?

—
Reply to this email directly or view it on GitHub
#489 (comment).

[rossberg]

@titzer, as I said, I see how that motivates strictness, but not a particular choice of operand/evaluation order, no?

[titzer]

Ah, right. We could make it strict and that fixes the problem as well.

On Thu, Dec 3, 2015 at 5:26 PM, rossberg-chromium notifications@github.com
wrote:

@titzer https://github.com/titzer, as I said, I see how that motivates
strictness, but not a particular choice of evaluation order, no?

—
Reply to this email directly or view it on GitHub
#489 (comment).

[sunfishcode]

@rossberg-chromium When translating wasm to machine code, there are many situations where the condition operand can be folded with the branch, such as folding a load into a cmp instruction on x86. In contrast, it's not clear that there are any situations where the result value operand can be folded, because it's just a value that needs to exist somewhere when the branch happens. A simple JIT that does pattern matching on expression trees alone, rather than constructing something like SSA form on the local variables, will be able to see more opportunities if we evaluate the result value operand first, because that will put the condition operand in an advantageous position in the expression tree more often.

[sunfishcode]

Friendly ping :-). This PR proposes reordering select's operands to put the condition last. This allows it to evaluate the condition last under the common evaluation rules, which is expected to enable more optimization opportunities.

[ghost]

The select change seems ideal to me. If there is multi-value support in future then it might look like (select (values (exp1) (exp2)) (...) (pred)) so there need be no issue with the number of arguments changing.

The br_if change also seems fine, but as a suggestion if the return value for br_if is optional and comes first then it looks like a br_if-block that could be extended to more than one expression, for example (br_if (block ... ... ...) (pred) <label>) or (br_if_block ... ... ... (pred) <label>) or perhaps just make it an option for block (block ... ... ... br_if (pred) <label>). Again if there are multiple values in future then this could be (br_if (values (exp1) (exp2)) (pred) <label>).

Was changing br and case to return a value intended?

[sunfishcode]

The changes to br and case were introduced as discussed above and just reflect what's already in the spec. There is also a change to put br_if's condition operand last, for the same reason as select.

[ghost]

@sunfishcode Perhaps the point of noting that br does not 'yield' a value is that it is not valid to use it in a place expecting a value such as (i32.add (br $l1 (...)) (...)), rather br causes the target form to yield the value.

[rossberg]

Nit: This may be a slightly inaccurate description, given that it allows fall-through.

[rossberg]

@JSStats, to be sure, a break is valid in that position, but it doesn't "yield a value" in the sense that it doesn't return one to the enclosing expression -- simply because it doesn't return to it at all. At least I think that was the intention of the wording.

[ghost]

@rossberg-chromium Thank you for the clarification. Is there a good reason to allow br in a context expecting a value? The patterns that come to mind seem to be more appropriately expressed with a block than an expression just for side effects. I can see that it could be useful on a branch of if_else that returns a value on the other branch.

[rossberg]

@JSStats, yes, it's useful exactly for control flow forks that return a value from their "regular" branches, but want to do a break on some "exceptional" branch. A simple conditional is one example, a switch perhaps is a more interesting one.

[sunfishcode]

@rossberg-chromium Thanks for the correction; I misunderstood the intent of that wording. I've now removed that wording change, leaving the original text about br, case, and friends' return values in place.

This PR now just has the operand ordering change for select and br_if/tableswitch result operands, following the discussion in #469.

[titzer]

LGTM

[sunfishcode]

Merging with LGTM's above.