Closed Bug 1260737 Opened 4 years ago Closed 4 years ago

wasm: implement Reinterpret opcodes

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: bbouvier, Assigned: bbouvier)

References

Details

Attachments

(1 file)

No description provided.
Here we go!
Attachment #8736336 - Flags: review?(luke)
Jan, see above patch: I'm worried about the boilerplate for i64. Reinterpret can:

- have no int64 operands or result value (f32 -> i32, i32 -> f32)
- the result value can be an int64, but not the operands (f64 -> i64)
- the operand can be int64, but not the result value (i64 -> f64)

I've made a LIR opcode for each of the three above cases (because the number of Defs/Ops differs from one case to the other, with respect to INT64_PIECES), but this obviously doesn't scale. Another way would be to have a variadic number of defs/ops, but this sounds more complex. Would you recommend another way that would reduce boilerplate here?
Flags: needinfo?(jdemooij)
Comment on attachment 8736336 [details] [diff] [review]
reinterpret.patch

Review of attachment 8736336 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

::: js/src/jit/Lowering.cpp
@@ +2480,5 @@
> +    } else if (ins->input()->type() == MIRType_Int64) {
> +        define(new(alloc()) LAsmReinterpretFromI64(useInt64RegisterAtStart(ins->input())), ins);
> +    } else {
> +        define(new(alloc()) LAsmReinterpret(useRegisterAtStart(ins->input())), ins);
> +    }

If all the branches are single-line, SM style lets you avoid braces.
Attachment #8736336 - Flags: review?(luke) → review+
(In reply to Benjamin Bouvier [:bbouvier] from comment #2)
> Jan, see above patch: I'm worried about the boilerplate for i64. Reinterpret
> can:

I think 3 different LIR instructions to handle these cases isn't too bad..

We had a similar issue with Value, and for that I added useBoxOrTypedOrConstant. The idea is that you always have 2 'uses' on 32-bit, but you can either use both (Value or int64) or just one of them (typed or constant). We could do something similar for int64.
Flags: needinfo?(jdemooij)
https://hg.mozilla.org/mozilla-central/rev/ef04d22a4de5
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Nice, this makes box2d noticeably faster: 2.5% faster in throughput, 2.2% smaller in binary size, and 6% faster in startup ( https://github.com/WebAssembly/binaryen/pull/286#issuecomment-204601749 ). The pre-wasm pattern to reintepret values is painful, and box2d happens to hit it due to some LLVM issues.
You need to log in before you can comment on or make changes to this bug.