1678043 - Loop backedge jumps to the start of the loop alignment nops, not to the end of it

Status: RESOLVED FIXED

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

Description

[Lars T Hansen [:lth]]

Test case:

var ins = new WebAssembly.Instance(new WebAssembly.Module(wasmTextToBinary(`
  (module
    (func (export "f") (param $count i32) (result i32)
      (local $sum i32)
      (loop $L
        (local.set $sum (i32.add (i32.const 1) (local.get $sum)))
        (local.tee $count (i32.sub (local.get $count) (i32.const 1)))
        (br_if $L))
      (local.get $sum)))
`)));
wasmDis(ins.exports.f, "ion");

x64 code:

00000024  33 c0                     xor %eax, %eax
00000026  0f 1f 00                  nopl %eax, (%rax)
00000029  0f 1f 80 00 00 00 00      nopl %eax, (%rax)
00000030  41 83 7e 38 00            cmpl $0x00, 0x38(%r14)
00000035  0f 84 02 00 00 00         jz 0x000000000000003D
0000003B  0f 0b                     ud2
0000003D  83 c0 01                  add $0x01, %eax
00000040  83 ef 01                  sub $0x01, %edi
00000043  85 ff                     test %edi, %edi
00000045  75 df                     jnz 0x0000000000000026

Observe that the jnz jumps to the first nop, not to after the last nop, which is where it should jump. (The nops are the loop alignment.)

Comment 1

[Lars T Hansen [:lth]]

This is an Ion bug; baseline gets it right.

Comment 2

[Lars T Hansen [:lth]]

Attachment: Bug 1678043 - Place nop-alignment for loop before the label. r?nbp

The nopAlign needs to come before the loop header is bound in Ion code.
I don't know if this bug has been there all along or was the result of
a later merge problem, but anyway it seems straightforward enough.

Depends on D97542

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2b460b0ea61
Place nop-alignment for loop before the label.  r=nbp

Comment 4

[Sandor Molnar[:smolnar]]

Backed out for causing arm64 bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/cc6b9d58862f9a03b2223b3c696e8fcaba5ba94b

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunning%2Cpending%2Crunnable&revision=a2b460b0ea61f4aed0f5ad85cf477555bbf76fee&selectedTaskRun=TYZZU40jSjC60YsMh9Bugg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=322296805&repo=autoland&lineNumber=11026

TEST-PASS | js/src/jit-test/tests/sunspider/check-string-fasta.js | Success (code 0, args "") [7.0 s]
[task 2020-11-19T13:15:05.697Z] WARNING: VIXL simulator support for load-/store-/clear-exclusive instructions is limited. Refer to the README for details.
[task 2020-11-19T13:15:05.697Z] /builds/worker/checkouts/gecko/js/src/jit-test/tests/wasm/atomic.js line 185 > WebAssembly.Module:63:1 RuntimeError: unaligned memory access
[task 2020-11-19T13:15:05.697Z] Stack:
[task 2020-11-19T13:15:05.697Z] @/builds/worker/checkouts/gecko/js/src/jit-test/tests/wasm/atomic.js:350:18
[task 2020-11-19T13:15:05.697Z] Exit code: 3
[task 2020-11-19T13:15:05.697Z] FAIL - wasm/atomic.js
[task 2020-11-19T13:15:05.697Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/wasm/atomic.js | WARNING: VIXL simulator support for load-/store-/clear-exclusive instructions is limited. Refer to the README for details. (code 3, args "--wasm-compiler=baseline") [0.6 s]
[task 2020-11-19T13:15:05.698Z] INFO exit-status : 3
[task 2020-11-19T13:15:05.698Z] INFO timed-out : False
[task 2020-11-19T13:15:05.698Z] INFO stderr 2> WARNING: VIXL simulator support for load-/store-/clear-exclusive instructions is limited. Refer to the README for details.
[task 2020-11-19T13:15:05.698Z] INFO stderr 2> /builds/worker/checkouts/gecko/js/src/jit-test/tests/wasm/atomic.js line 185 > WebAssembly.Module:63:1 RuntimeError: unaligned memory access
[task 2020-11-19T13:15:05.698Z] INFO stderr 2> Stack:
[task 2020-11-19T13:15:05.698Z] INFO stderr 2> @/builds/worker/checkouts/gecko/js/src/jit-test/tests/wasm/atomic.js:350:18
[task 2020-11-19T13:15:05.833Z] TEST-PASS | js/src/jit-test/tests/sunspider/check-access-fannkuch.js | Success (code 0, args "") [11.3 s]

Comment 5

[Lars T Hansen [:lth]]

Bug was not here but in bug 1678049.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dd15d7dcec77
Place nop-alignment for loop before the label.  r=nbp

Comment 7

[Atila Butkovits]

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