[PATCH] [Reassociation] Add support for reassociation with unsafe algebra.

[Chandler Carruth]

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:362
@@ +361,3 @@
+    Res = BinaryOperator::CreateFNeg(S1, Name, InsertBefore);
+    Res->setFastMathFlags(cast<FPMathOperator>(FlagsOp)->getFastMathFlags());
+  }
----------------
Why a local variable above but not here? I don't care deeply about which pattern you use, but I would generally avoid the single-use variable unless things get truly terrible to read...

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:854
@@ -801,1 +853,3 @@
+      if (NewOp->getType()->isFPOrFPVectorTy())
+        NewOp->setFastMathFlags(I->getFastMathFlags());
     } else {
----------------
The more I see of code like this, the more I think we need a richer clone approach in instructions... but that's not for this patch...

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:901-902
@@ +900,4 @@
+            "Unexpected constant.");
+    return isa<ConstantInt>(V) ? ConstantExpr::getNeg(C) :
+      ConstantExpr::getFNeg(C);
+  }
----------------
This formatting I find hard to read... clang-format?

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:914
@@ -848,2 +913,3 @@
   //
-  if (BinaryOperator *I = isReassociableOp(V, Instruction::Add)) {
+  BinaryOperator *I = isReassociableOp(V, Instruction::Add, Instruction::FAdd);
+  if (I) {
----------------
Are you actually intending to use this variable outside of the if? How (as its null)? See comment below...

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:963
@@ -896,3 +962,3 @@
   // negation.
-  return BinaryOperator::CreateNeg(V, V->getName() + ".neg", BI);
+  return CreateNeg(V, V->getName() + ".neg", BI, BI);
 }
----------------
You pass BI twice here... is that intentional? Was one of them supposed to be I?

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:1951-1952
@@ -1847,2 +1950,4 @@
 
-    return;
+    // Don't try to optimize floating point vector instructions or anything
+    // that doesn't have unsafe algebra.
+    if (I->getType()->isVectorTy() || !I->hasUnsafeAlgebra())
----------------
Why not floating point vector instructions? That seems rather odd to group with testing for fastmath...

http://reviews.llvm.org/D4129