Open Bug 1057779 Opened 10 years ago Updated 2 years ago

IonMonkey: Unsigned comparisons

Categories

(Core :: JavaScript Engine: JIT, enhancement, P5)

enhancement

Tracking

()

People

(Reporter: sunfish, Unassigned)

Details

Attachments

(1 file)

Attached patch unsigned-comparisons.patch (deleted) — 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 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)
Dan, want to get back to this at some point?
Flags: needinfo?(sunfish)
Priority: -- → P5
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)
Assignee: sunfish → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: