Closed
Bug 1260737
Opened 8 years ago
Closed 8 years ago
wasm: implement Reinterpret opcodes
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(1 file)
29.83 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 2•8 years ago
|
||
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)
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7920778de50e
Comment 4•8 years ago
|
||
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+
Comment 6•8 years ago
|
||
(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)
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ef04d22a4de5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 8•8 years ago
|
||
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.
Description
•