Skip to content

ARM: select on an i64-comparison condition with COMPUTED arms always returns the then-arm — reload clobbers the live else-arm register #973

Description

@avrabe

Summary

On ARM (--target cortex-m4 --relocatable, direct selector), a select whose
condition is an i64 comparison and whose arms are computed values
(not constants) always returns the then-arm. The else-arm's register is
spilled and then reloaded with the then-arm's value, so both it arms move
the same register.

The bare i64 comparison is correct on its own — only the select around it is
wrong. RV32 is unaffected (its bytes for the same fixture are identical and its
oracle is green).

Found by the #970 lane's byte-identity sweep over scripts/repro/*.wat, which
compiled every fixture for both backends. rv32_cmp_select_472.wat is an
RV32 fixture; nothing in CI compiles it for ARM, so no oracle covers this.

This is PRE-EXISTING and independent of #970: the same 44/49 vectors match
wasmtime on the binary built before and after the #970 fix, with the
identical five failures. The #970 change is byte-neutral for this function.

Reproduction (executed)

(module
  (func (export "sel_i64_lt_s") (param i32 i32) (result i32)
    (select (i32.add (local.get 0) (i32.const 100))
            (i32.add (local.get 1) (i32.const 200))
            (i64.lt_s (i64.extend_i32_s (local.get 0))
                      (i64.extend_i32_s (local.get 1)))))
  (func (export "cmp_i64_lt_s") (param i32 i32) (result i32)
    (i64.lt_s (i64.extend_i32_s (local.get 0))
              (i64.extend_i32_s (local.get 1)))))
synth compile i64sel.wat --target cortex-m4 --relocatable --all-exports -o i64sel.o

unicorn (UC_ARCH_ARM/UC_MODE_THUMB) vs wasmtime:

ok  sel_i64_lt_s(5,7):  want=105 got=105
BUG sel_i64_lt_s(7,5):  want=205 got=107
BUG sel_i64_lt_s(0,0):  want=200 got=100
ok  sel_i64_lt_s(-1,1): want=99  got=99
BUG sel_i64_lt_s(1,-1): want=199 got=101
ok  cmp_i64_lt_s(...)   5/5 correct   <- the comparison ALONE is fine

Every failing vector returns a+100 (the then-arm) where the else-arm was due.
Replacing the arms with i32.const makes it pass — the arms must be computed
into registers for the bug to appear.

Root cause, from the emitted Thumb-2

0008: add.w r3, r0, #0x64      ; then-arm  = a + 100
000c: add.w r5, r1, #0xc8      ; else-arm  = b + 200      -> r5
0010: mov   r6, r0
0012: asr.w r7, r6, #0x1f
0016: str.w r3, [sp]           ; SPILL the then-arm
001a: mov   r2, r1
001c: asr.w r3, r2, #0x1f
0020: cmp   r6, r2
0022: sbcs.w r4, r7, r3        ; the 64-bit compare
0026: ite   lt
0028: movlt r4, #1
002a: movge r4, #0
002c: ldr.w r5, [sp]           ; RELOAD the then-arm INTO r5 — clobbers else-arm
0030: cmp   r4, #0
0032: it    ne
0034: movne r6, r5
0036: it    eq
0038: moveq r6, r5             ; both arms now move the SAME register

The i64 comparison's register pressure (it needs a second pair for the
sign-extended halves) forces a spill of the then-arm operand, and the reload
picks r5 — the register still holding the live else-arm operand. The
else-arm value is destroyed before the it/mov pair consumes it.

The movne/moveq pair reading the same source register is a local,
checkable invariant
: a select's two conditional moves must never have the
same source. That looks like a cheap defensive assertion in the selector, on
top of whatever fixes the reload's interference.

Suggested gate

An ARM execution differential over select with an i64-comparison condition
and computed arms (the shape above), plus the movne/moveq-same-source
structural assertion. scripts/repro/rv32_cmp_select_472.wat already contains
the shape as sel_cmp_i64; it is only ever compiled for RV32.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions