Don't require IMT thunk alignment

VERIFIED FIXED

Status

VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: edwsmith, Unassigned)

Tracking

unspecified
x86
All
Bug Flags:
flashplayer-triage +

Details

(Reporter)

Description

10 years ago
Pointers to IMT thunks currently must be 8-byte aligned because we tag the pointers in AvmCore::makeITrampBinding().  This requires the jit to support aligning method entries.  which requires inserting nop's.  which, if done ideally, requires cpu detection because certian i686 implementations (notably Crusoe and Via) don't support them.  

easier if we just avoid pointer tagging and alignment completely.

see bug 473552 for details.

Comment 1

10 years ago
as a temporary band-aid, can we emit universally legal nop sequences as 473552 suggests?
(Reporter)

Comment 2

10 years ago
if this were urgent for tamarin, yes.  but it isn't, so I'd rather just fix it right.

Comment 3

10 years ago
Ed, we should consider memmove to do the alignment. The nops might not be entirely free in tight loops (the might be free for method alignment though, due to call overhead).
(Reporter)

Comment 4

10 years ago
alignment for performance purposes, like a loop header:  yes, agreed.

alignment so that a function pointer has some extra bits for tagging:  annoying, need to do tagging another way, and decouple from how code is generated.

Updated

10 years ago
Flags: flashplayer-triage+
Flags: flashplayer-qrb?

Comment 5

10 years ago
Where is this alignment done? (I'm working on a separate change that will eliminate makeITrampBinding as a side effect, and might as well fix this while I'm at it is simple...)
(Reporter)

Comment 6

10 years ago
this has already been fixed, closing.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

10 years ago
for posterity's sake, Tamarin used to take the code pointer for an IMT thunk, and | it with BKIND_ITRAMP.  This requires the entry point address to be 8-aligned.

Since then, we now create a MethodInfo for every piece of code (methods and IMT thunks), and the MethodInfo points to the code with no tagging.  We tag the MethodInfo* with BKIND_ITRAMP, now.

this was done to reduce differences between VMCFG_METHODENV_IMPL32=1 and =0, clean up bits of code touching imt MethodEnv's, and to remove this code-alignment requirement.

Updated

9 years ago
Status: RESOLVED → VERIFIED

Comment 8

9 years ago
removing QRB request, bug resolved/verified
Flags: flashplayer-qrb?
You need to log in before you can comment on or make changes to this bug.