Closed
Bug 710948
Opened 12 years ago
Closed 12 years ago
IonMonkey: visitCallGeneric() should be platform-independent.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: sstangl, Assigned: sstangl)
References
Details
Attachments
(1 file)
28.70 KB,
patch
|
mjrosenb
:
review+
|
Details | Diff | Splinter Review |
The visitCallGeneric() function currently has a separate implementation on ARM. The logic is very nearly the same as on x86/x86_64: with a small amount of effort, we can abstract away the differences and have only a single implementation, which is obviously desirable. We require ARM CallVM support to do this, since bug 708441 uses CallVM from visitCallGeneric() to handle uncompiled/native functions. Once that lands we can merge. There are probably a few other code generator functions that also should be merged.
Assignee | ||
Comment 1•12 years ago
|
||
First attempt at evicting visitCallGeneric(). Appears to pass tests without being too whiny. This brings ARM up to the x86/x64 level of call support, finally.
Attachment #584678 -
Flags: review?(mrosenberg)
Comment 2•12 years ago
|
||
Comment on attachment 584678 [details] [diff] [review] Make visitCallGeneric() platform-independent, v1 Review of attachment 584678 [details] [diff] [review]: ----------------------------------------------------------------- You should file a bug about load16. ::: js/src/ion/CodeGenerator.cpp @@ +494,5 @@ > + masm.bind(&rejoin); > + } > + > + // Increment to remove IonFramePrefix; decrement to fill FrameSizeClass. > + int prefixGarbage = 2 * sizeof(void *); this number should be 3 * sizeof(void *) on arm. I think sizeof(IonJSFrameLayout) - sizeof(void*) is a platform independent way of calculating this. I have no clue how this code is not throwing assertion failures left and right ::: js/src/ion/LIR-Common.h @@ -245,5 @@ > const LDefinition &tmpobjreg) > : argslot_(argslot) > { > setOperand(0, func); > - setTemp(0, token); I am concerned about the disappearance of a temp register, I assume you found a way to multiplex the existing two temp registers in order to get the same effect. ::: js/src/ion/arm/MacroAssembler-arm.cpp @@ +1007,5 @@ > + > +void > +MacroAssemblerARMCompat::callIon(const Register &callee) > +{ > + ma_callIon(callee); This should update framePushed, since the net effect is sp+=4.
Attachment #584678 -
Flags: review?(mrosenberg) → review+
Comment 3•12 years ago
|
||
Just noticed that the |visitCallGeneric()| still has two calls to |callIon()|, which arguably is wrong. If you don't fix it before you push, I have it fixed locally.
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Marty Rosenberg [:Marty] from comment #2) > I am concerned about the disappearance of a temp register, I assume you > found a way to multiplex the existing two temp registers in order to get the > same effect. Yes. Also eliminated 2 unnecessary movePtr instructions in the process.
Assignee | ||
Comment 5•12 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/0371557c1ec4
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•