Closed
Bug 875419
Opened 11 years ago
Closed 11 years ago
fold ImmWord immediates into 32-bit immediate fields when possible on x64
Categories
(Core :: JavaScript Engine, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: sunfish, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
3.00 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
The x64 MacroAssembler has overloads of storePtr, cmpPtr, and addPtr which take n ImmWord immediate. Currently, it unconditionally loads the immediate value into a temporary register.
Reporter | ||
Comment 1•11 years ago
|
||
Attachment #753387 -
Flags: review?(sstangl)
Reporter | ||
Comment 2•11 years ago
|
||
Note that this patch depends on the patch in 875413.
Comment 3•11 years ago
|
||
Comment on attachment 753387 [details] [diff] [review] a proposed fix Review of attachment 753387 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/x64/MacroAssembler-x64.h @@ +387,5 @@ > cmpq(lhs, ScratchReg); > } > void cmpPtr(const Operand &lhs, const ImmWord rhs) { > + if ((intptr_t)rhs.value <= INT32_MAX && > + (intptr_t)rhs.value >= INT32_MIN) { nits: trailing whitespace, and if the conditional of an if)( spans multiple lines, our style guide requires the "{" on a new line. Since putting the >= clause on the same line as <= does not violate the 100-colmun limit, we can just do that. @@ +447,5 @@ > addq(imm, Operand(dest)); > } > void addPtr(ImmWord imm, const Register &dest) { > JS_ASSERT(dest != ScratchReg); > + if ((intptr_t)imm.value <= INT32_MAX && and here @@ +563,5 @@ > loadPtr(src, dest); > shlq(Imm32(1), dest); > } > void storePtr(ImmWord imm, const Address &address) { > + if ((intptr_t)imm.value <= INT32_MAX && and here :)
Attachment #753387 -
Flags: review?(sstangl) → review+
Reporter | ||
Comment 4•11 years ago
|
||
Fixed whitespace per review comments.
Attachment #753387 -
Attachment is obsolete: true
Attachment #753483 -
Flags: review?(sstangl)
Updated•11 years ago
|
Attachment #753483 -
Flags: review?(sstangl) → review+
Updated•11 years ago
|
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed71811445d0
Keywords: checkin-needed
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ed71811445d0
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•