Closed
Bug 468445
Opened 16 years ago
Closed 15 years ago
Nanojit support for PowerPC64
Categories
(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: edwsmith, Assigned: edwsmith)
References
Details
Attachments
(3 obsolete files)
32bit PPC port is done (for mac at least), now lets do PPC64 * requires general 64bit fixes in LIR * new instructions for handling pointers * optimizations, eg int32->double, uint32->double
Comment 1•16 years ago
|
||
PPC 64bit buildbot slave has been added and will be activated once this is pushed.
Flags: flashplayer-triage+
Flags: flashplayer-qrb?
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → edwsmith
Assignee | ||
Comment 2•16 years ago
|
||
work in progress, with hg branches for blocking patches http://hg.mozilla.org/users/edwsmith_adobe.com/ppc64
Assignee | ||
Comment 3•16 years ago
|
||
This is what remains after factoring out earlier patches. It needs to be split into 2+ smaller patches, but I'm posting now for early feedback. I am tempted to isolate the generic LIR 64bit changes from the PPC/x64 backend changes, even though the 64bit-specific opcodes won't be exersized yet (in tamarin, at least). if someone has a better suggestion on how to factor this patch please chime in.
Assignee | ||
Comment 4•16 years ago
|
||
Add many more 64bit lir opcodes, and new arg types for CallInfo signatures This patch just does the LIR groundwork for 64bit backends, without adding any new backends. - rename ARGSIZE_LO to ARGSIZE_I, add ARGSIZE_U. these represent 32bit args, and the I and U are a hint for when they must be promoted to 64bit values on a 64bit ABI. - add ARGSIZE_P alias, for pointer arguments - add ARGSIZE_V for functions that return void - define new more detailed signatures for all the functions in vm_fops.h CodegenLIR was heavily modified to distinguish between int32, uint32, and pointer values. For pointer values, aliased LIR ops are used to pick either the 32bit or 64bit operation required. values of type Atom are generally treated as pointers, except in the few cases when we know what we're doing with them (eg creating a boolean atom from a bool). Verifier now calls emitPtrConst() for const pointers and atoms, instead of calling emitIntConst(). MIR's emitIntConst() is now just a wrapper for emitPtrConst(). new LIR opcodes required by Tamarin: LIR_qursh: 64bit unsigned right shift LIR_qirsh: 64bit signed right shift LIR_qaddp: 64bit non-cse pointer add LIR_qxor: 64bit xor LIR_i2q: sign extend 32->64 LIR_u2q: zero extend 32->64 LIR_qeq: 64bit int equals LIR_qle: 64bit signed <= LIR_qlt: 64bit signed < LIR_qge: 64bit signed >= LIR_qgt: 64bit signed > LIR_qule: 64bit unsigned <= LIR_qult: 64bit unsigned < LIR_quge: 64bit unsigned >= LIR_qugt: 64bit unsigned > new ops that replace old ones LIR_iaddp replaces LIR_addp: 32bit pointer add LIR_qcall replaces LIR_callh on 64bit. 64bit GP call LIR_icall replaces LIR_call: 32bit GP call. LIR_callh is still used on 32bit builds to represent the MSW 32bit result from a 64bit call. Not for use at all on 64bit builds these ops are mutually exclusive (i used on 32bit builds and q used on 64bit builds): LIR_[iq]alloc, LIR_[iq]call, LIR_[iq]param. isS24() macro had to be fixed to properly sign extend on 64bit builds... a case of misplaced parens, it turns out.
Attachment #356974 -
Attachment is obsolete: true
Attachment #357154 -
Flags: review?(gal)
Assignee | ||
Comment 5•16 years ago
|
||
supports 64bit lir instructions when NANOJIT_64BIT is defined, plus a few other 64bit tweaks required in the assembler.
Attachment #357155 -
Flags: review?(rreitmai)
Assignee | ||
Updated•16 years ago
|
Attachment #357154 -
Flags: review?(rreitmai)
Comment 6•16 years ago
|
||
Comment on attachment 357154 [details] [diff] [review] 64bit upgrades for LIR notation (no new backends) - Nativei386/PPC.cpp - asm_promote TODO() - CodegenLIR.cpp:1960 do we need to assert for 64b builds that the args are aligned or is this already assumed in the emitter. - CodegenLIR.cp:2169 do we even support/test a build without WRITE_BARRIERS? Maybe the #error should be moved to include other non-supported ifdefs - CodegenLIR.cpp:3817 binaryIns(LIR_lt), mind to double check this optimization, it looks a little odd, although I think its carried over verbatim from MIR so its probably correct. - CodegenLIR.cpp:3959 is _save_eip really 32b on 64b platforms? - CodegenLIR:cpp:3983 was this a 32b bug also? And a general comment for which I don't have a good answer, but it strikes me that we now have a LIR for which one is not sure what instructions require support on 32b and 64b platforms. For example, LIR_qalloc and LIR_ialloc are both available in 32b builds but only ialloc is used. Others such as LIR_i/u2q and LIR_qlo are even more ambiguous; should a 32bit back-end support them? Perhaps we can ifdef AVMPLUS_64BIT around the LIR instructions themselves to distinguish what is needed for each of the builds. Also, I didn't look at all instances but I expect that Assembler should not be using any of the LIR aliases; this would only throw more confusion to the issue.
Attachment #357154 -
Flags: review?(rreitmai) → review+
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #6) > (From update of attachment 357154 [details] [diff] [review]) > - Nativei386/PPC.cpp - asm_promote TODO() never called for 32bit backends. This patch (357154) does not yet enable either of the 64bit backends. > - CodegenLIR.cpp:1960 do we need to assert > for 64b builds that the args are aligned or > is this already assumed in the emitter. This is code that generates args for the VM abi, and the vm assumes everywhere that they're 8-byte aligned. the semantics of LIR_alloc guarantee 8-byte alignment for any size >= 8 bytes. (an assert would be hard, it would have to be in the generated code, not in the generator). > - CodegenLIR.cp:2169 do we even support/test > a build without WRITE_BARRIERS? > Maybe the #error should be moved to include > other non-supported ifdefs I agree. I will create a patch that precedes this LIR patch, that just makes these combinations produce errors, early on, and clean up this code. > - CodegenLIR.cpp:3817 binaryIns(LIR_lt), mind > to double check this optimization, it looks a little odd, > although I think its carried over verbatim from MIR > so its probably correct. good catch, its wrong. for it to work the two values have to be promoted first and we need a quad-size compare. And, it should be AVMPLUS_64BIT not AMD64. > - CodegenLIR.cpp:3959 is _save_eip really 32b > on 64b platforms? good catch, another bug. save_eip is indeed 32bit values (pc offsets, not actual pointers) but debugEnter expects a pointer to sintptr, not a pointer to int32_t. Need to fix this up. > - CodegenLIR:cpp:3983 was this a 32b bug also? > > And a general comment for which I don't have a > good answer, but it strikes me that we now have > a LIR for which one is not sure what instructions > require support on 32b and 64b platforms. > > For example, LIR_qalloc and LIR_ialloc are both > available in 32b builds but only ialloc is used. > Others such as LIR_i/u2q and LIR_qlo are > even more ambiguous; should a 32bit back-end > support them? the answer depends on both the ABI and the frontend. In principle there's nothing wrong with a 32bit jit that wants to use the 64bit int operations; however obviously the backend will have to support those ops either via lowering them during LIR generation, or by supporting them at assembly time. Asserts will catch unsupported opcodes either way. > Perhaps we can ifdef AVMPLUS_64BIT around > the LIR instructions themselves to distinguish > what is needed for each of the builds. > > Also, I didn't look at all instances but I expect that > Assembler should not be using any of the LIR aliases; > this would only throw more confusion to the issue. The aliases will no doubt be a source of trouble. we can avoid using them, the only sure fire thing to do is remove them, and rely on helper functions for generating them, and sniffer functions for testing for them. both add overhead. in places where you really want the constant value of a LIR instruction, and want the alias, and dont want to be fooled (eg in case statements), maybe macros are what we want. they decorate the usage to avoid confusion but still boil down to the appropriate constant. eg use PTR_SIZE(LIR_ialloc,LIR_palloc) expllicitly, instead of an alias LIR_alloc.
Comment 8•16 years ago
|
||
Comment on attachment 357155 [details] [diff] [review] ppc64 backend, requires lir64.patch Marking + without thoroughly reviewing the ppc specific code, let me know if you'd like something more extensive. But I'm surmising that passing the acceptance tests in this case is clear enough indication the code is quite alright. One observation though: - NativePPC underrunProtect() has br value 4 for 32b and 7 for 64b ; just checking that it shouldn't be 8?
Attachment #357155 -
Flags: review?(rreitmai) → review+
Assignee | ||
Comment 9•16 years ago
|
||
(In reply to comment #8) > (From update of attachment 357155 [details] [diff] [review]) > Marking + without thoroughly reviewing the ppc specific > code, let me know if you'd like something more extensive. > But I'm surmising that passing the acceptance tests in > this case is clear enough indication the code is quite alright. > > One observation though: > - NativePPC underrunProtect() has br value 4 for 32b and > 7 for 64b ; just checking that it shouldn't be 8? 7 is correct. the worst case branch code that constructs a 64bit immediate const to jump to, takes 7 instructions. ppc64 (and ppc32, maybe) should make use of a constant pool and put absolute addresses in it, however since it has no PC-relative addressing, we'll need a dedicated register to point to const data near the code, and that's too much for this patch.
Comment 10•16 years ago
|
||
Comment on attachment 357154 [details] [diff] [review] 64bit upgrades for LIR notation (no new backends) Comment from david: would be nice to split LIR_float and LIR_quad too
Attachment #357154 -
Flags: review?(gal) → review+
Assignee | ||
Comment 11•16 years ago
|
||
Comment on attachment 357154 [details] [diff] [review] 64bit upgrades for LIR notation (no new backends) pushed to tamarin-redux changeset: 1313:de598a38ae61
Attachment #357154 -
Attachment is obsolete: true
Assignee | ||
Comment 12•16 years ago
|
||
Comment on attachment 357155 [details] [diff] [review] ppc64 backend, requires lir64.patch pushed to tamarin-redux changeset: 1315:841135282486
Attachment #357155 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•