Split compare instruction handling OUT of handleTwoArgFP into handleCompareFP.
authorChris Lattner <sabre@nondot.org>
Fri, 11 Jun 2004 04:25:06 +0000 (04:25 +0000)
committerChris Lattner <sabre@nondot.org>
Fri, 11 Jun 2004 04:25:06 +0000 (04:25 +0000)
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).

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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@14139 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Target/X86/FloatingPoint.cpp
lib/Target/X86/X86FloatingPoint.cpp

index 329cdd8e7519ee1d2be51697935d63e38c1a21bd..0a8f4ac47866a9a42b62a635f58d9bac2d5d655c 100644 (file)
@@ -145,6 +145,7 @@ namespace {
     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 @@ bool FPS::processBasicBlock(MachineFunction &MF, MachineBasicBlock &BB) {
     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 @@ bool FPS::processBasicBlock(MachineFunction &MF, MachineBasicBlock &BB) {
       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 @@ static const TableEntry ForwardST0Table[] = {
   { 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 @@ static const TableEntry ReverseST0Table[] = {
   { 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 @@ static const TableEntry ForwardSTiTable[] = {
   { 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 @@ static const TableEntry ReverseSTiTable[] = {
   { X86::FpDIV  , X86::FDIVrST0 },
   { X86::FpMUL  , X86::FMULrST0 },
   { X86::FpSUB  , X86::FSUBrST0 },
-  { X86::FpUCOM , ~0            },
-  { X86::FpUCOMI, ~0            },
 };
 
 
@@ -523,11 +518,6 @@ static const TableEntry ReverseSTiTable[] = {
 ///         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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
   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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
     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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
       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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
 
   // 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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
     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
 }
 
index 329cdd8e7519ee1d2be51697935d63e38c1a21bd..0a8f4ac47866a9a42b62a635f58d9bac2d5d655c 100644 (file)
@@ -145,6 +145,7 @@ namespace {
     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 @@ bool FPS::processBasicBlock(MachineFunction &MF, MachineBasicBlock &BB) {
     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 @@ bool FPS::processBasicBlock(MachineFunction &MF, MachineBasicBlock &BB) {
       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 @@ static const TableEntry ForwardST0Table[] = {
   { 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 @@ static const TableEntry ReverseST0Table[] = {
   { 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 @@ static const TableEntry ForwardSTiTable[] = {
   { 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 @@ static const TableEntry ReverseSTiTable[] = {
   { X86::FpDIV  , X86::FDIVrST0 },
   { X86::FpMUL  , X86::FMULrST0 },
   { X86::FpSUB  , X86::FSUBrST0 },
-  { X86::FpUCOM , ~0            },
-  { X86::FpUCOMI, ~0            },
 };
 
 
@@ -523,11 +518,6 @@ static const TableEntry ReverseSTiTable[] = {
 ///         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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
   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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
     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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
       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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
 
   // 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 @@ void FPS::handleTwoArgFP(MachineBasicBlock::iterator &I) {
     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
 }