704454 - Assertion failure: ((offset) % 4) == 0 (../nanojit/NativeARM.cpp:1628)

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P3)

Description

[Brent Baker]

Assertion failure: ((offset) % 4) == 0 (../nanojit/NativeARM.cpp:1628) when running ecma3/TypeConversion/e9_3_1_3_rt.as

This was introduced in:

changeset:   6939:ab76e2ab1678
user:        Lars T Hansen <[e-mail]>
date:        Mon Nov 21 13:28:26 2011 +0100
summary:     Minor adjustments for issues uncovered in the review

Comment 1

[Brent Baker]

This actually asserts running anything that is jitted and NOT just TypeConversion testcase when running a debug build

Comment 2

[William Maddox]

The bug is due to this change in CodegenLIR.cpp:

- emitIns(BUILTIN_number,  ldd(subp(val, AtomConstants::kDoubleType), 0, ACCSET_OTHER) )
+            emitIns(BUILTIN_number,  ldd(val, -AtomConstants::kDoubleType, ACCSET_OTHER) )

The floating-point load and store instructions on ARM assume that the displacement is a multiple of four, and cannot encode other values.  The ARM code generator just passes the restriction through to LIR_ldd.  Since we expect LIR_ldd to respect natural alignment, this usually works, but the code is being clever here, using the displacement to subtract off the tag.

Comment 3

[William Maddox]

Attachment: Allow unaligned displacements to LIR_ldd/ldf/std/stf etc.

This patch relaxes the restriction on the displacement, at the expense of a potentially useful heuristic for detecting unaligned accesses.  It effectively undoes the clever tag-removal trick on ARM, but allows it to operate on other platforms such as x86 without cluttering the front-end or LIR semantics.

Comment 4

[Lars T Hansen]

(In reply to William Maddox from comment #2)

> ... but the
> code is being clever here, using the displacement to subtract off the tag.

You flatter - all I did was change the code to its most natural form.  Remarkable that we haven't seen this problem before.

(I wonder idly whether the ARM assembler has peephole optimization to fuse the two instructions that must have been generated by the original LIR into one, as it should have.)

Comment 5

[William Maddox]

(In reply to Lars T Hansen from comment #4)

> (I wonder idly whether the ARM assembler has peephole optimization to fuse
> the two instructions that must have been generated by the original LIR into
> one, as it should have.)

It cannot do so in general, because the load and store instructions are encoded
with two implicit zero low-order bits.

Comment 6

[Lars T Hansen]

Comment on attachment 576411 [details] [diff] [review]
Allow unaligned displacements to LIR_ldd/ldf/std/stf etc.

To my eye the logic looks exactly right (rematerialize the explicit offset calculation when needed). I'm not all that familiar with ARM or the ARM assembler's conventions though, so issues on that level could have eluded me.

Comment 7

[Edwin Smith]

Comment on attachment 576411 [details] [diff] [review]
Allow unaligned displacements to LIR_ldd/ldf/std/stf etc.

If there's any chance IP can be clobberred by underrunProtect generating a jump between code hunks, then we will need underrunProtect before sequences that use IP as scratch register, e.g. here:

+                FLDD(r, IP, 0);
+                asm_add_imm(IP, FP, d);

If underrunProtect cannot clobber IP then we're ok.  (and in the future could make IP available to the register allocator).

Comment 8

[William Maddox]

http://asteam.macromedia.com/hg/tr-float/rev/232377fad4ab