Bug 869507 - Eliminate unnecessary NaN checks. r=me
authorDan Gohman <sunfish@google.com>
Wed, 08 May 2013 16:38:39 -0700
changeset 142264 c1ee14175d13d0a1b67fc91d88ea01fa508ece92
parent 142263 f6b920f5e96c048539e2fcbab44a22079b339dd5
child 142265 e22a2019d323d39065c2ae9bdc704ca71b53e132
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs869507
milestone23.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 869507 - Eliminate unnecessary NaN checks. r=me Several common floating-point conditions can be implemented without using a separate NaN test.
js/src/ion/shared/Assembler-x86-shared.h
js/src/ion/shared/BaselineIC-x86-shared.cpp
js/src/ion/shared/CodeGenerator-x86-shared.h
js/src/ion/shared/MacroAssembler-x86-shared.h
--- a/js/src/ion/shared/Assembler-x86-shared.h
+++ b/js/src/ion/shared/Assembler-x86-shared.h
@@ -96,58 +96,67 @@ class AssemblerX86Shared
         DoubleNotEqualOrUnordered = NotEqual | DoubleConditionBitSpecial,
         DoubleGreaterThanOrUnordered = Below | DoubleConditionBitInvert,
         DoubleGreaterThanOrEqualOrUnordered = BelowOrEqual | DoubleConditionBitInvert,
         DoubleLessThanOrUnordered = Below,
         DoubleLessThanOrEqualOrUnordered = BelowOrEqual
     };
 
     enum NaNCond {
-        NaN_Unexpected,
+        NaN_HandledByCond,
         NaN_IsTrue,
         NaN_IsFalse
     };
 
+    // If the primary condition returned by ConditionFromDoubleCondition doesn't
+    // handle NaNs properly, return NaN_IsFalse if the comparison should be
+    // overridden to return false on NaN, NaN_IsTrue if it should be overridden
+    // to return true on NaN, or NaN_HandledByCond if no secondary check is
+    // needed.
     static inline NaNCond NaNCondFromDoubleCondition(DoubleCondition cond) {
         switch (cond) {
           case DoubleOrdered:
-          case DoubleEqual:
           case DoubleNotEqual:
           case DoubleGreaterThan:
           case DoubleGreaterThanOrEqual:
           case DoubleLessThan:
           case DoubleLessThanOrEqual:
-            return NaN_IsFalse;
           case DoubleUnordered:
           case DoubleEqualOrUnordered:
-          case DoubleNotEqualOrUnordered:
           case DoubleGreaterThanOrUnordered:
           case DoubleGreaterThanOrEqualOrUnordered:
           case DoubleLessThanOrUnordered:
           case DoubleLessThanOrEqualOrUnordered:
+            return NaN_HandledByCond;
+          case DoubleEqual:
+            return NaN_IsFalse;
+          case DoubleNotEqualOrUnordered:
             return NaN_IsTrue;
         }
 
         JS_NOT_REACHED("Unknown double condition");
-        return NaN_Unexpected;
+        return NaN_HandledByCond;
     }
 
     static void staticAsserts() {
         // DoubleConditionBits should not interfere with x86 condition codes.
         JS_STATIC_ASSERT(!((Equal | NotEqual | Above | AboveOrEqual | Below |
                             BelowOrEqual | Parity | NoParity) & DoubleConditionBits));
     }
 
     AssemblerX86Shared()
       : enoughMemory_(true)
     {
     }
 
     static Condition InvertCondition(Condition cond);
 
+    // Return the primary condition to test. Some primary conditions may not
+    // handle NaNs properly and may therefore require a secondary condition.
+    // Use NaNCondFromDoubleCondition to determine what else is needed.
     static inline Condition ConditionFromDoubleCondition(DoubleCondition cond) {
         return static_cast<Condition>(cond & ~DoubleConditionBits);
     }
 
     static void TraceDataRelocations(JSTracer *trc, IonCode *code, CompactBufferReader &reader);
 
     // MacroAssemblers hold onto gcthings, so they are traced by the GC.
     void trace(JSTracer *trc);
--- a/js/src/ion/shared/BaselineIC-x86-shared.cpp
+++ b/js/src/ion/shared/BaselineIC-x86-shared.cpp
@@ -24,17 +24,17 @@ ICCompare_Double::Compiler::generateStub
 
     Assembler::DoubleCondition cond = JSOpToDoubleCondition(op);
     masm.compareDouble(cond, FloatReg0, FloatReg1);
     masm.setCC(Assembler::ConditionFromDoubleCondition(cond), dest);
     masm.movzxbl(dest, dest);
 
     // Check for NaN, if needed.
     Assembler::NaNCond nanCond = Assembler::NaNCondFromDoubleCondition(cond);
-    if (nanCond != Assembler::NaN_Unexpected) {
+    if (nanCond != Assembler::NaN_HandledByCond) {
       masm.j(Assembler::NoParity, &notNaN);
       masm.mov(Imm32(nanCond == Assembler::NaN_IsTrue), dest);
       masm.bind(&notNaN);
     }
 
     masm.tagValue(JSVAL_TYPE_BOOLEAN, dest, R0);
     EmitReturnFromIC(masm);
 
--- a/js/src/ion/shared/CodeGenerator-x86-shared.h
+++ b/js/src/ion/shared/CodeGenerator-x86-shared.h
@@ -60,17 +60,17 @@ class CodeGeneratorX86Shared : public Co
 
     Operand createArrayElementOperand(Register elements, const LAllocation *index);
 
     void emitCompare(MCompare::CompareType type, const LAllocation *left, const LAllocation *right);
 
     // Emits a branch that directs control flow to the true block if |cond| is
     // true, and the false block if |cond| is false.
     void emitBranch(Assembler::Condition cond, MBasicBlock *ifTrue, MBasicBlock *ifFalse,
-                    Assembler::NaNCond ifNaN = Assembler::NaN_Unexpected);
+                    Assembler::NaNCond ifNaN = Assembler::NaN_HandledByCond);
     void emitBranch(Assembler::DoubleCondition cond, MBasicBlock *ifTrue, MBasicBlock *ifFalse);
 
     bool emitTableSwitchDispatch(MTableSwitch *mir, const Register &index, const Register &base);
 
   public:
     CodeGeneratorX86Shared(MIRGenerator *gen, LIRGraph *graph, MacroAssembler *masm);
 
   public:
--- a/js/src/ion/shared/MacroAssembler-x86-shared.h
+++ b/js/src/ion/shared/MacroAssembler-x86-shared.h
@@ -418,24 +418,24 @@ class MacroAssemblerX86Shared : public A
             break;
           default:
             return false;
         }
         return true;
     }
 
     void emitSet(Assembler::Condition cond, const Register &dest,
-                 Assembler::NaNCond ifNaN = Assembler::NaN_Unexpected) {
+                 Assembler::NaNCond ifNaN = Assembler::NaN_HandledByCond) {
         if (GeneralRegisterSet(Registers::SingleByteRegs).has(dest)) {
             // If the register we're defining is a single byte register,
             // take advantage of the setCC instruction
             setCC(cond, dest);
             movzxbl(dest, dest);
 
-            if (ifNaN != Assembler::NaN_Unexpected) {
+            if (ifNaN != Assembler::NaN_HandledByCond) {
                 Label noNaN;
                 j(Assembler::NoParity, &noNaN);
                 if (ifNaN == Assembler::NaN_IsTrue)
                     movl(Imm32(1), dest);
                 else
                     xorl(dest, dest);
                 bind(&noNaN);
             }