While working on bug 1073928, I noticed that the range computed for constants greater than INT32_MAX or less than INT32_MIN was unnecessarily broad. The attached patch fixes it.

One more patch; I noticed that beta nodes for comparisons like |x < 0| would generate beta nodes that included negative zero on both sides. Negative zero is not actually equal to zero, so we can exclude it on the false side.

Oh, one more one more thing. We can support JSOP_STRICTEQ and JSOP_STRICTNE with beta nodes too.

Comment on attachment 8516797 [details] [diff] [review] range-strict-cmp.patch Review of attachment 8516797 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/RangeAnalysis.cpp @@ +251,4 @@ > case JSOP_EQ: > comp.setDouble(bound, bound); > break; > + case JSOP_STRICTNE: What about an object that overrides its "valueOf" to -0? if (obj !== 0) { assertEq (obj == -0, false); // Will assert. obj equals to -0. }

The range-non-int32-constant.patch patch: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e8c12e04a14

Comment on attachment 8516797 [details] [diff] [review] range-strict-cmp.patch (In reply to Hannes Verschore [:h4writer] from comment #3) > What about an object that overrides its "valueOf" to -0? > if (obj !== 0) { > assertEq (obj == -0, false); // Will assert. obj equals to -0. > } JSOP_STRICTEQ can call valueOf while JSOP_EQ can't? In that case, I'll just drop this patch.

(In reply to Dan Gohman [:sunfish] from comment #5) > Comment on attachment 8516797 [details] [diff] [review] > range-strict-cmp.patch > > (In reply to Hannes Verschore [:h4writer] from comment #3) > > What about an object that overrides its "valueOf" to -0? > > if (obj !== 0) { > > assertEq (obj == -0, false); // Will assert. obj equals to -0. > > } > > JSOP_STRICTEQ can call valueOf while JSOP_EQ can't? In that case, I'll just > drop this patch. The issue is that on STRICT compare "obj !== 0" is always true, since the types don't match. Even if ToNumber(obj) is 0 or -0.

(In reply to Hannes Verschore [:h4writer] from comment #6) > The issue is that on STRICT compare "obj !== 0" is always true, since the > types don't match. Even if ToNumber(obj) is 0 or -0. Can we do a strict compare if the types of the operands are not MIRType_Object, or is there something else easy we can do? The patch eliminates a few instructions in stanford-crypto-sha256-iterative, for example, so it'd be neat if there were a way to fix it.

Is this what you suggested earlier? It's possible I misunderstood, since I'm unfamiliar with the semantics of strict comparisons.

Comment on attachment 8519096 [details] [diff] [review] range-strict-cmp.patch Review of attachment 8519096 [details] [diff] [review]: ----------------------------------------------------------------- That looks correct. You can even increase that list to: > Compare_Int32, > Compare_Int32MaybeCoerceBoth, > Compare_Int32MaybeCoerceLHS, > Compare_Int32MaybeCoerceRHS, > > Compare_Double, > > Compare_DoubleMaybeCoerceLHS, > Compare_DoubleMaybeCoerceRHS, > > Compare_Float Please add this test in a functions to MCompare ;) or a static function. Also don't forget to add testcases that test this. E.g. for placeses where this would be incorrect.

Cool. I've updated the patch accordingly.

Comment on attachment 8521470 [details] [diff] [review] range-strict-cmp.patch Review of attachment 8521470 [details] [diff] [review]: ----------------------------------------------------------------- Awesome, thanks!