Open
Bug 1057779
Opened 10 years ago
Updated 2 years ago
IonMonkey: Unsigned comparisons
Categories
(Core :: JavaScript Engine: JIT, enhancement, P5)
Core
JavaScript Engine: JIT
Tracking
()
NEW
People
(Reporter: sunfish, Unassigned)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
IonMonkey has code to convert double comparisons into int32 comparisons when the inputs can be converted to int32. This patch adds similar code for uint32.
This eliminates the last remaining floating-point operations in the hot loop in the SJCL SHA-512 benchmark -- http://jsperf.com/sha-512/3.
Attachment #8477920 -
Flags: review?(nicolas.b.pierron)
Comment 1•10 years ago
|
||
Comment on attachment 8477920 [details] [diff] [review]
unsigned-comparisons.patch
Review of attachment 8477920 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/jit/RangeAnalysis.cpp
@@ +2597,5 @@
> + } else if (input->isUrsh() &&
> + IsZero(input->getOperand(1)) &&
> + input->getOperand(0)->type() == MIRType_Int32)
> + {
> + JS_ASSERT(input->range()->isUInt32());
style-nit: MOZ_ASSERT.
@@ +2598,5 @@
> + IsZero(input->getOperand(1)) &&
> + input->getOperand(0)->type() == MIRType_Int32)
> + {
> + JS_ASSERT(input->range()->isUInt32());
> + truncated->replaceOperand(i, input->getOperand(0));
This sounds fine for MCompare, because we only truncate if both outputs are in the uint32 range. And this also sounds fine for other truncating operations as they would be converting their inputs to fit in the int32 range. The only thing I wonder about is indirectly truncating operators. I guess this does not matter for operations such as add/sub/multiply but it does for modulo:
((((a >>> 1) - 1) >>> 0) % 3) | 0
cannot be replaced by
(((a >>> 1) - 1) % 3) | 0
Attachment #8477920 -
Flags: review?(nicolas.b.pierron)
Comment 2•8 years ago
|
||
Dan, want to get back to this at some point?
Flags: needinfo?(sunfish)
Priority: -- → P5
Reporter | ||
Comment 3•8 years ago
|
||
Yes, would be good to get back to at some point. The main challenge is the subtlety in the range analysis, but it should be doable.
Flags: needinfo?(sunfish)
Updated•3 years ago
|
Assignee: sunfish → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•