Closed Bug 1639486 Opened 3 months ago Closed 3 months ago

Fix baseline compilation of fac-ssa

Categories

(Core :: Javascript: WebAssembly, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: wingo, Assigned: wingo)

References

Details

Attachments

(1 file)

Failing test case:

wasmFullPass(`
  ;; Iterative factorial without locals.
  (func $pick0 (param i64) (result i64 i64)
    (local.get 0) (local.get 0)
  )
  (func $pick1 (param i64 i64) (result i64 i64 i64)
    (local.get 0) (local.get 1) (local.get 0)
  )
  (func (export "run") (param i64) (result i64)
    (i64.const 1) (local.get 0)
    (loop $l (param i64 i64) (result i64)
      (call $pick1) (call $pick1) (i64.mul)
      (call $pick1) (i64.const 1) (i64.sub)
      (call $pick0) (i64.const 0) (i64.gt_u)
      (br_if $l)
      (drop) (return)
    )
  )`,
             7034535277573963776n, {}, 25n);

Rather confusingly, sometimes stack height refers to a height with stack
results, and sometimes a height without results. We were confusing the
two: the raw shuffleStackResultsTowardFP function that took uint32_t
heights assumed the former, but the version that took StackHeight
assumed the latter. The result was a mis-shuffle that could be
catastrophic.

Besides fixing that bug, in this patch we rename all variables that
denote heights without stack results to be "base" -- e.g. srcBase
instead of srcHeight, and so on. Hopefully this convention minimizes
confusion risk in the future.

I tried to have a look at providing two StackHeight types, but I
couldn't see a way that would provide meaningful invariants -- you need
to be able to convert between the two in a number of cases, so that
attempt appeared to be ceremony without benefit.

Pushed by wingo@igalia.com:
https://hg.mozilla.org/integration/autoland/rev/a1e751f0ab22
Fix shuffling of stack results in br_if / br_table r=lth
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.