Closed Bug 524230 Opened 15 years ago Closed 15 years ago

NJ merge: adjust asm_ld_imm assertion

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: graydon, Assigned: graydon)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Assembler::asm_ld_imm in NativeARM.cpp does the following:

- Writes the literal to *(_nSlot++)
- Writes an offset-load to the literal to *(--_nIns)

Explanation:
------------

- _nSlot is at a lower address and grows upwards towards
  _nIns

- _nIins is at a higher address and grows downwards towards
  _nSlot

- they grow together, and can get as close as touching, but
  must not pass each other

- the instruction being written is a 12-bit offset from "PC"
  packed into a 32-bit word

- PC points to "current instruction + 8"

- but offset is calculated from _nIns - 2. IOW offset is
  just (_nSlot - _nIns)


Diagram:
--------

       imm       load
 [____|____|____|____|____|____|____]
      ^              ^
     nslot          nins


Difference in code:
-------------------

Our code differs from Adobe's only in an assert:

-    NanoAssert(isS12(offset) && (offset < 0));
+    NanoAssert(isS12(offset) && (offset < -8));

Clearly the first assert is not right, as the offset needs
to be at least -8. The second assert seems a little too
tight though, as I think an offset of exactly -8 would still
function, and is probably achievable in rare cases. So I'd
set it to <= -8. But I'm willing to hear an argument to the
contrary.

Any opinions?
Discussed and reviewed on IRC, set to <= -8, tested, seems ok. Worst case it's just a wrong assert and won't harm any production builds anyway.

http://hg.mozilla.org/tracemonkey/rev/47fddf8e644c

Adobe folks: please either absorb this back or tell us how our logic's off here :)
Whiteboard: fixed-in-tracemonkey
Sounds good to me, will change tamarin code.
sounds ok
http://hg.mozilla.org/mozilla-central/rev/47fddf8e644c
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.