Fix invalid X87 stack depth after LCompareNumericAndBranch.

src/ia32/lithium-codegen-ia32.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 397 matching lines @@
     if (!CpuFeatures::IsSupported(SSE2)) FlushX87StackIfNecessary(instr);
 
     RecordAndUpdatePosition(instr->position());
 
     instr->CompileToNative(this);
 
     if (!CpuFeatures::IsSupported(SSE2)) {
       if (instr->IsGoto()) {
         x87_stack_.LeavingBlock(current_block_, LGoto::cast(instr));
       } else if (FLAG_debug_code && FLAG_enable_slow_asserts &&
                  !instr->IsGap() && !instr->IsReturn()) {

[oliv, 2013/09/24 09:03:15]

It would be nice to document the invariant this cl restores, which is afaik:

if (instr->ClobbersDoubleRegisters()) {
  ASSERT(x87_stack_.depth() ==
         instr->HasDoubleRegisterResult() ? 1 : 0);
}

[Benedikt Meurer, 2013/09/24 09:26:48]

Done.

         __ VerifyX87StackDepth(x87_stack_.depth());
       }
     }
   }
   EnsureSpaceForLazyDeopt();
   return !is_aborted();
 }
 
 
 bool LCodeGen::GenerateJumpTable() {
@@ skipping 125 matching lines @@
 }
 
 
 void LCodeGen::X87LoadForUsage(X87Register reg) {
   ASSERT(x87_stack_.Contains(reg));
   x87_stack_.Fxch(reg);
   x87_stack_.pop();
 }
 
 
+void LCodeGen::X87LoadForUsage(X87Register reg1, X87Register reg2) {
+  ASSERT(x87_stack_.Contains(reg1));
+  ASSERT(x87_stack_.Contains(reg2));
+  x87_stack_.Fxch(reg1, 1);
+  x87_stack_.Fxch(reg2);
+  x87_stack_.pop();
+  x87_stack_.pop();

[Toon Verwaest, 2013/09/24 09:00:08]

Pretty surprising that you first push values to them immediately pop. Can't we
let FCmp consume these values to avoid direct dependency between these
instructions?

[Benedikt Meurer, 2013/09/24 09:12:30]

As discussed offline, we can fix this once the X87 stack tracking is part of the
macro assembler.

+}
+
+
 void LCodeGen::X87Stack::Fxch(X87Register reg, int other_slot) {
   ASSERT(is_mutable_);
   ASSERT(Contains(reg) && stack_depth_ > other_slot);
   int i  = ArrayIndex(reg);
   int st = st2idx(i);
   if (st != other_slot) {
     int other_i = st2idx(other_slot);
     X87Register other = stack_[other_i];
     stack_[other_i]   = reg;
     stack_[i]         = other;
@@ skipping 1991 matching lines @@
     double right_val = ToDouble(LConstantOperand::cast(right));
     int next_block = EvalComparison(instr->op(), left_val, right_val) ?
         instr->TrueDestination(chunk_) : instr->FalseDestination(chunk_);
     EmitGoto(next_block);
   } else {
     if (instr->is_double()) {
       if (CpuFeatures::IsSafeForSnapshot(SSE2)) {
         CpuFeatureScope scope(masm(), SSE2);
         __ ucomisd(ToDoubleRegister(left), ToDoubleRegister(right));
       } else {
-        X87Fxch(ToX87Register(right));
-        X87Fxch(ToX87Register(left), 1);
-        __ fld(0);
-        __ fld(2);
+        X87LoadForUsage(ToX87Register(right), ToX87Register(left));
         __ FCmp();
       }
       // Don't base result on EFLAGS when a NaN is involved. Instead
       // jump to the false block.
       __ j(parity_even, instr->FalseLabel(chunk_));
     } else {
       if (right->IsConstantOperand()) {
         __ cmp(ToOperand(left),
                ToImmediate(right, instr->hydrogen()->representation()));
       } else if (left->IsConstantOperand()) {
@@ skipping 3805 matching lines @@
                               FixedArray::kHeaderSize - kPointerSize));
   __ bind(&done);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32