Bug 869507 - Eliminate unnecessary NaN checks. r=me
authorDan Gohman <sunfish@google.com>
Wed, 08 May 2013 16:38:39 -0700
changeset 138092 c1ee14175d13d0a1b67fc91d88ea01fa508ece92
parent 138091 f6b920f5e96c048539e2fcbab44a22079b339dd5
child 138093 e22a2019d323d39065c2ae9bdc704ca71b53e132
push id3752
push userlsblakk@mozilla.com
push dateMon, 13 May 2013 17:21:10 +0000
treeherdermozilla-aurora@1580544aef0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs869507
milestone23.0a1
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);
             }