[llvm-commits] CVS: llvm/lib/Target/X86/FloatingPoint.cpp

Chris Lattner lattner at cs.uiuc.edu
Thu Jun 10 23:31:01 PDT 2004


Changes in directory llvm/lib/Target/X86:

FloatingPoint.cpp updated: 1.33 -> 1.34

---
Log message:

Split compare instruction handling OUT of handleTwoArgFP into handleCompareFP.
This makes the code much simpler, and the two cases really do belong apart.
Once we do it, it's pretty obvious how flawed the logic was for A != A case,
so I fixed it (fixing PR369: http://llvm.cs.uiuc.edu/PR369 ).

This also uses freeStackSlotAfter instead of inserting an fxchg then
popStackAfter'ing in the case where there is a dead result (unlikely, but 
possible), producing better code.



---
Diffs of the changes:  (+49 -43)

Index: llvm/lib/Target/X86/FloatingPoint.cpp
diff -u llvm/lib/Target/X86/FloatingPoint.cpp:1.33 llvm/lib/Target/X86/FloatingPoint.cpp:1.34
--- llvm/lib/Target/X86/FloatingPoint.cpp:1.33	Wed Jun  2 00:55:25 2004
+++ llvm/lib/Target/X86/FloatingPoint.cpp	Thu Jun 10 23:25:06 2004
@@ -145,6 +145,7 @@
     void handleOneArgFP(MachineBasicBlock::iterator &I);
     void handleOneArgFPRW(MachineBasicBlock::iterator &I);
     void handleTwoArgFP(MachineBasicBlock::iterator &I);
+    void handleCompareFP(MachineBasicBlock::iterator &I);
     void handleCondMovFP(MachineBasicBlock::iterator &I);
     void handleSpecialFP(MachineBasicBlock::iterator &I);
   };
@@ -214,7 +215,12 @@
     case X86II::ZeroArgFP:  handleZeroArgFP(I); break;
     case X86II::OneArgFP:   handleOneArgFP(I);  break;  // fstp ST(0)
     case X86II::OneArgFPRW: handleOneArgFPRW(I); break; // ST(0) = fsqrt(ST(0))
-    case X86II::TwoArgFP:   handleTwoArgFP(I);  break;
+    case X86II::TwoArgFP:
+      if (I->getOpcode() != X86::FpUCOM && I->getOpcode() != X86::FpUCOMI)
+        handleTwoArgFP(I);
+      else
+        handleCompareFP(I);
+      break;
     case X86II::CondMovFP:  handleCondMovFP(I); break;
     case X86II::SpecialFP:  handleSpecialFP(I); break;
     default: assert(0 && "Unknown FP Type!");
@@ -226,10 +232,7 @@
       unsigned Reg = IB->second;
       if (Reg >= X86::FP0 && Reg <= X86::FP6) {
 	DEBUG(std::cerr << "Register FP#" << Reg-X86::FP0 << " is dead!\n");
-	++I;                         // Insert fxch AFTER the instruction
-	moveToTop(Reg-X86::FP0, I);  // Insert fxch if necessary
-	--I;                         // Move to fxch or old instruction
-	popStackAfter(I);            // Pop the top of the stack, killing value
+        freeStackSlotAfter(I, Reg-X86::FP0);
       }
     }
     
@@ -481,8 +484,6 @@
   { X86::FpDIV  , X86::FDIVST0r },
   { X86::FpMUL  , X86::FMULST0r },
   { X86::FpSUB  , X86::FSUBST0r },
-  { X86::FpUCOM , X86::FUCOMr   },
-  { X86::FpUCOMI, X86::FUCOMIr  },
 };
 
 // ReverseST0Table - Map: A = B op C  into: ST(0) = ST(i) op ST(0)
@@ -491,8 +492,6 @@
   { X86::FpDIV  , X86::FDIVRST0r },
   { X86::FpMUL  , X86::FMULST0r  },   // commutative
   { X86::FpSUB  , X86::FSUBRST0r },
-  { X86::FpUCOM , ~0             },
-  { X86::FpUCOMI, ~0             },
 };
 
 // ForwardSTiTable - Map: A = B op C  into: ST(i) = ST(0) op ST(i)
@@ -501,8 +500,6 @@
   { X86::FpDIV  , X86::FDIVRrST0 },
   { X86::FpMUL  , X86::FMULrST0  },   // commutative
   { X86::FpSUB  , X86::FSUBRrST0 },
-  { X86::FpUCOM , X86::FUCOMr    },
-  { X86::FpUCOMI, X86::FUCOMIr   },
 };
 
 // ReverseSTiTable - Map: A = B op C  into: ST(i) = ST(i) op ST(0)
@@ -511,8 +508,6 @@
   { X86::FpDIV  , X86::FDIVrST0 },
   { X86::FpMUL  , X86::FMULrST0 },
   { X86::FpSUB  , X86::FSUBrST0 },
-  { X86::FpUCOM , ~0            },
-  { X86::FpUCOMI, ~0            },
 };
 
 
@@ -523,11 +518,6 @@
 ///         ST(i) = fsub  ST(0), ST(i)
 ///         ST(0) = fsubr ST(0), ST(i)
 ///         ST(i) = fsubr ST(0), ST(i)
-///
-/// In addition to three address instructions, this also handles the FpUCOM
-/// instruction which only has two operands, but no destination.  This
-/// instruction is also annoying because there is no "reverse" form of it
-/// available.
 /// 
 void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
   ASSERT_SORTED(ForwardST0Table); ASSERT_SORTED(ReverseST0Table);
@@ -535,10 +525,7 @@
   MachineInstr *MI = I;
 
   unsigned NumOperands = MI->getNumOperands();
-  bool isCompare = MI->getOpcode() == X86::FpUCOM ||
-                   MI->getOpcode() == X86::FpUCOMI;
-  assert((NumOperands == 3 || (NumOperands == 2 && isCompare)) &&
-	 "Illegal TwoArgFP instruction!");
+  assert(NumOperands == 3 && "Illegal TwoArgFP instruction!");
   unsigned Dest = getFPReg(MI->getOperand(0));
   unsigned Op0 = getFPReg(MI->getOperand(NumOperands-2));
   unsigned Op1 = getFPReg(MI->getOperand(NumOperands-1));
@@ -550,11 +537,6 @@
     KillsOp1 |= (KI->second == X86::FP0+Op1);
   }
 
-  // If this is an FpUCOM instruction, we must make sure the first operand is on
-  // the top of stack, the other one can be anywhere...
-  if (isCompare)
-    moveToTop(Op0, I);
-
   unsigned TOS = getStackEntry(0);
 
   // One of our operands must be on the top of the stack.  If neither is yet, we
@@ -579,7 +561,7 @@
       Op0 = TOS = Dest;
       KillsOp0 = true;
     }
-  } else if (!KillsOp0 && !KillsOp1 && !isCompare) {
+  } else if (!KillsOp0 && !KillsOp1) {
     // If we DO have one of our operands at the top of the stack, but we don't
     // have a dead operand, we must duplicate one of the operands to a new slot
     // on the stack.
@@ -590,8 +572,7 @@
 
   // Now we know that one of our operands is on the top of the stack, and at
   // least one of our operands is killed by this instruction.
-  assert((TOS == Op0 || TOS == Op1) &&
-	 (KillsOp0 || KillsOp1 || isCompare) && 
+  assert((TOS == Op0 || TOS == Op1) && (KillsOp0 || KillsOp1) && 
 	 "Stack conditions not set up right!");
 
   // We decide which form to use based on what is on the top of the stack, and
@@ -628,22 +609,47 @@
     popStackAfter(I);   // Pop the top of stack
   }
 
-  // Insert an explicit pop of the "updated" operand for FUCOM 
-  if (isCompare) {
-    if (KillsOp0 && !KillsOp1)
-      popStackAfter(I);   // If we kill the first operand, pop it!
-    else if (KillsOp1 && Op0 != Op1)
-      freeStackSlotAfter(I, Op1);
-  }
-      
   // Update stack information so that we know the destination register is now on
   // the stack.
-  if (!isCompare) {  
-    unsigned UpdatedSlot = getSlot(updateST0 ? TOS : NotTOS);
-    assert(UpdatedSlot < StackTop && Dest < 7);
-    Stack[UpdatedSlot]   = Dest;
-    RegMap[Dest]         = UpdatedSlot;
+  unsigned UpdatedSlot = getSlot(updateST0 ? TOS : NotTOS);
+  assert(UpdatedSlot < StackTop && Dest < 7);
+  Stack[UpdatedSlot]   = Dest;
+  RegMap[Dest]         = UpdatedSlot;
+  delete MI;   // Remove the old instruction
+}
+
+/// handleCompareFP - Handle FpUCOM and FpUCOMI instructions, which have two FP
+/// register arguments and no explicit destinations.
+/// 
+void FPS::handleCompareFP(MachineBasicBlock::iterator &I) {
+  ASSERT_SORTED(ForwardST0Table); ASSERT_SORTED(ReverseST0Table);
+  ASSERT_SORTED(ForwardSTiTable); ASSERT_SORTED(ReverseSTiTable);
+  MachineInstr *MI = I;
+
+  unsigned NumOperands = MI->getNumOperands();
+  assert(NumOperands == 2 && "Illegal FpUCOM* instruction!");
+  unsigned Op0 = getFPReg(MI->getOperand(NumOperands-2));
+  unsigned Op1 = getFPReg(MI->getOperand(NumOperands-1));
+  bool KillsOp0 = false, KillsOp1 = false;
+
+  for (LiveVariables::killed_iterator KI = LV->killed_begin(MI),
+	 E = LV->killed_end(MI); KI != E; ++KI) {
+    KillsOp0 |= (KI->second == X86::FP0+Op0);
+    KillsOp1 |= (KI->second == X86::FP0+Op1);
   }
+
+  // Make sure the first operand is on the top of stack, the other one can be
+  // anywhere.
+  moveToTop(Op0, I);
+
+  // Replace the old instruction with a new instruction
+  MBB->remove(I++);
+  unsigned Opcode = MI->getOpcode() == X86::FpUCOM ? X86::FUCOMr : X86::FUCOMIr;
+  I = BuildMI(*MBB, I, Opcode, 1).addReg(getSTReg(Op1));
+
+  // If any of the operands are killed by this instruction, free them.
+  if (KillsOp0) freeStackSlotAfter(I, Op0);
+  if (KillsOp1 && Op0 != Op1) freeStackSlotAfter(I, Op1);
   delete MI;   // Remove the old instruction
 }
 





More information about the llvm-commits mailing list