Closed
Bug 1396342
Opened 7 years ago
Closed 7 years ago
Different behavior in wasm baseline/ion
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: azakai, Assigned: lth)
Details
Attachments
(2 files, 2 obsolete files)
376 bytes,
application/x-javascript
|
Details | |
1.46 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
Running these commands leads to different output: js --no-wasm-ion a.js t.wasm js --no-wasm-baseline a.js t.wasm calling: func_97 exception: RuntimeError: index out of bounds calling: func_97 result: 4294967296 The second result (without baseline) is what v8 also emits, so perhaps the issue is in baseline.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
A little more manual reduction leads to (module (export "func_97" (func $0)) (func $0 (result i32) (i64.gt_u (i64.const 1) (i64.div_s (i64.const -1) (i64.const 4294967296) ) ) ) ) Baseline returns 0, ion returns 1.
Assignee | ||
Comment 3•7 years ago
|
||
Alon, x64 Linux I assume?
Assignee: nobody → lhansen
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
This is a bug in the optimized divide-by-power-of-two path in emitQuotientI64. The problem seems to be that it uses Imm32(c-1) to adjust before shifting rather than Imm64(c-1), and that is wrong in this case. emitRemainderI64() uses Imm64(c-1).
Assignee | ||
Comment 5•7 years ago
|
||
Selfcontained test case, it should print 0 and 0 but prints -1 and -1 with the baseline compiler.
Attachment #8903983 -
Attachment is obsolete: true
Attachment #8903984 -
Attachment is obsolete: true
Assignee | ||
Comment 6•7 years ago
|
||
Emit the proper 64-bit add for the divide-by-power-of-two optimization. (Other cases look good.) Includes test case.
Attachment #8904214 -
Flags: review?(luke)
Updated•7 years ago
|
Attachment #8904214 -
Flags: review?(luke) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/60fd99ec9e71 Wasm baseline, fix division-by-64-bit constant bug. r=luke
Keywords: checkin-needed
Reporter | ||
Comment 8•7 years ago
|
||
(In reply to Lars T Hansen [:lth] from comment #3) > Alon, x64 Linux I assume? Yes, I saw this on x64 linux.
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/60fd99ec9e71
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•