diff -u -r1.4 -r1.4.2.3
--- include/llvm/Instruction.h	27 Oct 2002 13:44:47 -0000	1.4
+++ include/llvm/Instruction.h	27 Oct 2002 22:12:34 -0000	1.4.2.3
@@ -74,6 +74,24 @@
     return iType >= BinaryOpsBegin && iType < BinaryOpsEnd;
   }
 
+  static bool isCommutativeOp(unsigned op) {
+    switch (op) {
+    case Add:
+    case Mul:
+    case And: 
+    case Or:
+    case Xor:
+    case SetEQ:
+    case SetNE:
+      return true;
+    default:
+      return false;
+    }
+  }
+  bool isCommutative() const { return isCommutativeOp(getOpcode()); }
+
+  bool isAssociative() const;
+
   virtual void print(std::ostream &OS) const;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
--- lib/Analysis/ValueNumbering.cpp	27 Oct 2002 13:44:47 -0000	1.2
+++ lib/Analysis/ValueNumbering.cpp	27 Oct 2002 14:52:31 -0000	1.2.2.1
@@ -134,11 +134,7 @@
   //
   if ((I1.getOperand(0) == I2->getOperand(1) &&
        I1.getOperand(1) == I2->getOperand(0)) &&
-      (I1.getOpcode() == Instruction::Add || 
-       I1.getOpcode() == Instruction::Mul ||
-       I1.getOpcode() == Instruction::And || 
-       I1.getOpcode() == Instruction::Or  ||
-       I1.getOpcode() == Instruction::Xor))
+      I1.isCommutative())
     return true;
 
   return false;
--- lib/Transforms/Scalar/Reassociate.cpp	27 Oct 2002 13:44:51 -0000	1.3
+++ lib/Transforms/Scalar/Reassociate.cpp	31 Oct 2002 03:24:24 -0000	1.3.2.13
@@ -86,21 +86,14 @@
 }
 
 
-// isCommutativeOperator - Return true if the specified instruction is
+// isAssociativeOperator - Return true if the specified instruction is
 // commutative and associative.  If the instruction is not commutative and
 // associative, we can not reorder its operands!
 //
-static inline BinaryOperator *isCommutativeOperator(Instruction *I) {
-  // Floating point operations do not commute!
-  if (I->getType()->isFloatingPoint()) return 0;
-
-  if (I->getOpcode() == Instruction::Add || 
-      I->getOpcode() == Instruction::Mul ||
-      I->getOpcode() == Instruction::And || 
-      I->getOpcode() == Instruction::Or  ||
-      I->getOpcode() == Instruction::Xor)
+static inline BinaryOperator *isAssociativeOperator(Instruction *I) {
+  if (I->isAssociative())
     return cast<BinaryOperator>(I);
-  return 0;    
+  return 0;
 }
 
 
@@ -114,7 +107,8 @@
 
   // Make sure the LHS of the operand always has the greater rank...
   if (LHSRank < RHSRank) {
-    I->swapOperands();
+    bool success = !I->swapOperands();
+    assert(success && "swapOperands failed");
     std::swap(LHS, RHS);
     std::swap(LHSRank, RHSRank);
     Changed = true;
@@ -137,15 +131,28 @@
 
         // Convert ((a + 12) + 10) into (a + (12 + 10))
         I->setOperand(0, LHSI->getOperand(TakeOp));
-        LHSI->setOperand(TakeOp, RHS);
-        I->setOperand(1, LHSI);
+
+        // Move the LHS expression forward, to ensure that it is
+        // dominated by its operands.
+        std::string name = LHSI->getName();
+        LHSI->setName("");
+        BinaryOperator *NewLHS =
+          BinaryOperator::create(LHSI->getOpcode(),
+                                 LHSI->getOperand(0), LHSI->getOperand(1),
+                                 name, I);
+
+        NewLHS->setOperand(TakeOp, RHS);
+        I->setOperand(1, NewLHS);
+
+        assert(LHSI->use_size() == 0);
+        LHSI->getParent()->getInstList().erase(LHSI);
 
         ++NumChanged;
         DEBUG(std::cerr << "Reassociated: " << I << " Result BB: "
                         << I->getParent());
 
         // Since we modified the RHS instruction, make sure that we recheck it.
-        ReassociateExpr(LHSI);
+        ReassociateExpr(NewLHS);
         return true;
       }
     }
@@ -159,7 +166,7 @@
 // version of the value is returned, and BI is left pointing at the instruction
 // that should be processed next by the reassociation pass.
 //
-static Value *NegateValue(Value *V, BasicBlock *BB, BasicBlock::iterator &BI) {
+static Value *NegateValue(Value *V, BasicBlock::iterator& BI) {
   // We are trying to expose opportunity for reassociation.  One of the things
   // that we want to do to achieve this is to push a negation as deep into an
   // expression chain as possible, to expose the add instructions.  In practice,
@@ -171,8 +178,8 @@
   //
   if (Instruction *I = dyn_cast<Instruction>(V))
     if (I->getOpcode() == Instruction::Add && I->use_size() == 1) {
-      Value *RHS = NegateValue(I->getOperand(1), BB, BI);
-      Value *LHS = NegateValue(I->getOperand(0), BB, BI);
+      Value *RHS = NegateValue(I->getOperand(1), BI);
+      Value *LHS = NegateValue(I->getOperand(0), BI);
 
       // We must actually insert a new add instruction here, because the neg
       // instructions do not dominate the old add instruction in general.  By
@@ -187,12 +194,7 @@
   // Insert a 'neg' instruction that subtracts the value from zero to get the
   // negation.
   //
-  Instruction *Neg =
-    BinaryOperator::create(Instruction::Sub,
-                           Constant::getNullValue(V->getType()), V,
-                           V->getName()+".neg", BI);
-  --BI;
-  return Neg;
+  return BI = BinaryOperator::createNeg(V, V->getName() + ".neg", BI);
 }
 
 
@@ -200,10 +202,40 @@
   bool Changed = false;
   for (BasicBlock::iterator BI = BB->begin(); BI != BB->end(); ++BI) {
 
+    if (BI->getOpcode() == Instruction::Sub &&
+        BI->getOperand(0) != Constant::getNullValue(BI->getType())) {
+      // Convert a subtract into an add and a neg instruction... so that sub
+      // instructions can be commuted with other add instructions...
+      //
+      // Calculate the negative value of Operand 1 of the sub instruction...
+      // and set it as the RHS of the add instruction we just made...
+
+      std::string name = BI->getName();
+      BI->setName("");
+      Instruction *New =
+        BinaryOperator::create(Instruction::Add,
+                               BI->getOperand(0),
+                               BI->getOperand(1),
+                               name,
+                               BI);
+
+      // Everyone now refers to the add instruction...
+      BI->replaceAllUsesWith(New);
+
+      // Put the new add in the place of the subtract... deleting the subtract
+      BB->getInstList().erase(BI);
+
+      BI = New;
+      New->setOperand(1, NegateValue(New->getOperand(1), BI));
+
+      Changed = true;
+      DEBUG(std::cerr << "Negated: " << New << " Result BB: " << BB);
+    }
+
     // If this instruction is a commutative binary operator, and the ranks of
     // the two operands are sorted incorrectly, fix it now.
     //
-    if (BinaryOperator *I = isCommutativeOperator(BI)) {
+    if (BinaryOperator *I = isAssociativeOperator(BI)) {
       if (!I->use_empty()) {
         // Make sure that we don't have a tree-shaped computation.  If we do,
         // linearize it.  Convert (A+B)+(C+D) into ((A+B)+C)+D
@@ -234,31 +266,6 @@
         //
         Changed |= ReassociateExpr(I);
       }
-
-    } else if (BI->getOpcode() == Instruction::Sub &&
-               BI->getOperand(0) != Constant::getNullValue(BI->getType())) {
-      // Convert a subtract into an add and a neg instruction... so that sub
-      // instructions can be commuted with other add instructions...
-      //
-      Instruction *New = BinaryOperator::create(Instruction::Add,
-                                                BI->getOperand(0),
-                                                BI->getOperand(1),
-                                                BI->getName());
-      Value *NegatedValue = BI->getOperand(1);
-
-      // Everyone now refers to the add instruction...
-      BI->replaceAllUsesWith(New);
-
-      // Put the new add in the place of the subtract... deleting the subtract
-      BI = BB->getInstList().erase(BI);
-      BI = ++BB->getInstList().insert(BI, New);
-
-      // Calculate the negative value of Operand 1 of the sub instruction...
-      // and set it as the RHS of the add instruction we just made...
-      New->setOperand(1, NegateValue(NegatedValue, BB, BI));
-      --BI;
-      Changed = true;
-      DEBUG(std::cerr << "Negated: " << New << " Result BB: " << BB);
     }
   }
 
--- lib/VMCore/Instruction.cpp	17 Sep 2002 07:25:16 -0000	1.3
+++ lib/VMCore/Instruction.cpp	31 Oct 2002 02:59:16 -0000	1.3.4.4
@@ -97,3 +97,13 @@
   
   return 0;
 }
+
+bool Instruction::isAssociative() const {
+  if(getOpcode() == Add || getOpcode() == Mul ||
+     getOpcode() == And || getOpcode() == Or ||
+     getOpcode() == Xor) {
+    // Floating point operations do not associate!
+    return !getType()->isFloatingPoint();
+  }
+  return 0;
+}
--- lib/VMCore/iOperators.cpp	17 Sep 2002 07:25:16 -0000	1.3
+++ lib/VMCore/iOperators.cpp	27 Oct 2002 22:17:26 -0000	1.3.4.4
@@ -133,21 +133,13 @@
 // order dependant (SetLT f.e.) the opcode is changed.
 //
 bool BinaryOperator::swapOperands() {
-  if (SetCondInst *SCI = dyn_cast<SetCondInst>(this)) {
+  if (isCommutative())
+    ;
+  else if (SetCondInst *SCI = dyn_cast<SetCondInst>(this))
     iType = SCI->getSwappedCondition();
-    std::swap(Operands[0], Operands[1]);
-    return false;
-  }
-
-  switch (getOpcode()) {
-    // Instructions that don't need opcode modification
-  case Add: case Mul:
-  case And: case Xor:
-  case Or:
-    // Error on the side of caution
-  default:
+  else
     return true;
-  }
+
   std::swap(Operands[0], Operands[1]);
   return false;
 }
