Closed
Bug 701125
Opened 13 years ago
Closed 13 years ago
IonMonkey: moveValue needs a relocation marker
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: jandem, Assigned: dvander)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
9.32 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
Both x86 and x64. x86 handles this correctly in visitValue but not in moveValue. We still need to fix moveValue (and probably just call moveValue from visitValue, like we do on x64).
Assignee | ||
Comment 1•13 years ago
|
||
Great catch. x86 fix is a little gross - I could make it look like the x64 one as well.
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 573760 [details] [diff] [review] fix Review of attachment 573760 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/x64/MacroAssembler-x64.h @@ +131,5 @@ > movq(op, dest); > } > void moveValue(const Value &val, const Register &dest) { > movq(ImmWord((void *)val.asRawBits()), dest); > + if (val.isGCThing()) The x86 implementation in visitValue calls isMarkable instead of isGCThing. Do we need isGCThing? ::: js/src/ion/x86/MacroAssembler-x86.h @@ +107,5 @@ > + gc::Cell *cell = reinterpret_cast<gc::Cell *>(val.toGCThing()); > + movl(ImmGCPtr(cell), data); > + } else { > + movl(Imm32(jv.s.payload.i32), data); > + } Can we call this function from CodeGeneratorX86::visitValue? I agree that rewriting this to be more like the x64 version may be nice, but it doesn't matter too much so I will leave that up to you.
Attachment #573760 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 3•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/8919425a2efc (with a follow-up fix for nits)
Assignee | ||
Updated•13 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•