Bug 869507 - IonMonkey x86: Eliminate unnecessary NaN checks. r=nbp
☠☠ backed out by 948efc855b5b ☠ ☠
authorDan Gohman <sunfish@google.com>
Wed, 08 May 2013 11:28:55 -0700
changeset 142225 97163e4941f28603dee73b0c448729b6909b0ea9
parent 142224 0d2ce5b020cf783902fea06818dcebfd33257bc4
child 142226 db9527acf096b3ccdf58b0ec27ceed5810878b8a
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)
reviewersnbp
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 - IonMonkey x86: Eliminate unnecessary NaN checks. r=nbp 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/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/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);
             }