1639486 - Fix baseline compilation of fac-ssa

Status: RESOLVED FIXED

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

Description

[Andy Wingo [:wingo]]

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);

Comment 1

[Andy Wingo [:wingo]]

Attachment: Bug 1639486 - Fix shuffling of stack results in br_if / br_table

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1e751f0ab22
Fix shuffling of stack results in br_if / br_table r=lth

Comment 3

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/a1e751f0ab22