Closed
Bug 701993
Opened 13 years ago
Closed 12 years ago
IonMonkey: Implement LLoadSlotT on ARM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
7.98 KB,
patch
|
mjrosenb
:
review+
|
Details | Diff | Splinter Review |
These were added in bug 700303.
Assignee | ||
Updated•13 years ago
|
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•13 years ago
|
||
In addition to LStoreSlot{V,T}, we also need to add LStoreElement{V,T} and LLoadElement{V,T}. I'll get to this next week.
Assignee | ||
Comment 2•12 years ago
|
||
LoadSlotV, StoreSlotV and StoreSlotT were already implemented, so this just adds LoadSlotT.
Attachment #580629 -
Flags: review?(mrosenberg)
Assignee | ||
Updated•12 years ago
|
Summary: IonMonkey: Implement LStoreSlotV and LStoreSlotT on ARM → IonMonkey: Implement LLoadSlotT on ARM
Comment 3•12 years ago
|
||
Comment on attachment 580629 [details] [diff] [review] Patch Review of attachment 580629 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/arm/CodeGenerator-arm.cpp @@ +1290,5 @@ > Register base = ToRegister(load->input()); > int32 offset = load->mir()->slot() * sizeof(js::Value); > > if (load->mir()->type() == MIRType_Double) > + masm.loadInt32OrDouble(Operand(base, offset), ToFloatRegister(load->output())); When mir()->type() is MIRType_Double, can we really get a value that is an integer? ::: js/src/ion/arm/MacroAssembler-arm.cpp @@ +988,5 @@ > > void > MacroAssemblerARM::ma_vstr(FloatRegister src, const Operand &addr) > { > + ma_vdtr(IsStore, addr, src); I didn't already do that? whoops. @@ +1224,5 @@ > + Label notInt32, end; > + > + // If it's an int, convert it to double. > + ma_ldr(ToType(src), ScratchRegister); > + branchTestInt32(Assembler::NotEqual, ScratchRegister, ¬Int32); Are we sure the only two tags we'll ever see here are int and double? ::: js/src/ion/arm/MacroAssembler-arm.h @@ +488,5 @@ > } > > void boolValueToDouble(const ValueOperand &operand, const FloatRegister &dest); > void int32ValueToDouble(const ValueOperand &operand, const FloatRegister &dest); > + void loadInt32OrDouble(const Operand &src, const FloatRegister &dest); Do we already use the name "loadInt32OrDouble" on other architectures? It just seems like it could be better named.
Attachment #580629 -
Flags: review?(mrosenberg) → review+
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Marty Rosenberg [:Marty] from comment #3) > > When mir()->type() is MIRType_Double, can we really get a value that is an > integer? Yeah this information comes from TI and there the double type includes the int32 type. When storing an integer to a known-double slot we don't convert it to double so we have to handle integers when loading a known-double from memory. JM has similar code in place and see bug 705351 comment 0 for a testcase where this matters. > > Are we sure the only two tags we'll ever see here are int and double? Yes, when a value of another type is stored, TI/OSI will invalidate the script. > > Do we already use the name "loadInt32OrDouble" on other architectures? It > just seems like it could be better named. We use the name on x86/x64 too. Let me know if you have a better name and I can change it.
Assignee | ||
Comment 5•12 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/e43d4d0a2dcb
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•