Closed
Bug 736420
Opened 12 years ago
Closed 12 years ago
IonMonkey: Make multiplies more efficient on ARM
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: mjrosenb, Unassigned)
Details
Attachments
(1 file)
4.40 KB,
patch
|
jbramley
:
review+
|
Details | Diff | Splinter Review |
Make multiplies by a constant more efficient on ARM by changing (x * (1<<y)) into lsl dest, x, #y; cmp x (dst asr #y) and the known non-overflow case of (x* (1<<y | 1 << z)) into add dest, x, x lsl #(y-z); lsl dest, dest, #z. I can't think of any surefire way of detecting overflow in this case, but I may be able to get away with seeing if the next higher power of two overflows. I ran this optimization on sunspider and v8, but I didn't see a noticeable improvement, so for now this patch is at best a "make me feel better about the actual instructions that we generate" type of patch.
Attachment #606516 -
Flags: review?(Jacob.Bramley)
Comment 1•12 years ago
|
||
Comment on attachment 606516 [details] [diff] [review] /home/mrosenberg/patches/MoreMul-r0.patch Review of attachment 606516 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/arm/CodeGenerator-arm.cpp @@ +353,5 @@ > const LAllocation *lhs = ins->getOperand(0); > const LAllocation *rhs = ins->getOperand(1); > const LDefinition *dest = ins->getDef(0); > MMul *mul = ins->mir(); > + bool handled = false; This ought to be in a scope local to where it's used (such as after the 'default' case). For the cases that don't use it, 'handled' can still be false even though the code has been handled, and this might be confusing later. @@ +392,5 @@ > + handled = true; > + } else { > + uint32 shift_rest; > + JS_FLOOR_LOG2(shift_rest, rest); > + if ((1 << shift_rest) == rest) { Perhaps add a comment stating that this handles the case where the operand is a combination of two powers of two. @@ +401,5 @@ > + } > + } > + } else if (ToRegister(lhs) != ToRegister(dest)) { > + // To stay on the safe side, only optimize things that are a > + // power of 2. A more common case (I suspect) is multiplication by small numbers which aren't necessarily powers of two. These are quite easy to implement using ADD and Operand 2. Still, it isn't likely to bump the benchmarks much. @@ +408,3 @@ > JS_FLOOR_LOG2(shift, constant); > if ((1 << shift) == constant) { > + // dest = lhs * (2**shift) The '**' operator doesn't exist in C, so it's probably best not to use it in comments. @@ +411,2 @@ > masm.ma_lsl(Imm32(shift), ToRegister(lhs), ToRegister(dest)); > + // assert (lhs == dest >> shift) I don't see an assertion.
Attachment #606516 -
Flags: review?(Jacob.Bramley) → review+
Reporter | ||
Comment 2•12 years ago
|
||
landed: http://hg.mozilla.org/projects/ionmonkey/rev/bc22b7e24bc3
Reporter | ||
Comment 3•12 years ago
|
||
whoops (again)
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
•