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 :)
Sounds good to me, will change tamarin code.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.