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

[Chad Rosier]

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:362
@@ +361,3 @@
+    Res = BinaryOperator::CreateFNeg(S1, Name, InsertBefore);
+    Res->setFastMathFlags(cast<FPMathOperator>(FlagsOp)->getFastMathFlags());
+  }
----------------
Chandler Carruth wrote:
> 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...
I agree.  I'll take care of it.

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

================
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) {
----------------
Chandler Carruth wrote:
> Are you actually intending to use this variable outside of the if? How (as its null)? See comment below...
No.  I'll put this in the canonical form.  Thanks.

if (BinaryOperator *I = isReassociableOp(V, Instruction::Add, Instruction::FAdd)) {

}

================
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);
 }
----------------
Chandler Carruth wrote:
> You pass BI twice here... is that intentional? Was one of them supposed to be I?
Unfortunately, it's intentional (for now).  The 3rd argument in the insertion point and the 4th is where FastMathFlags should be copied from.  For CreateMul and CreateAdd these two arguments are always the same.  However, for FNeg theres one instance where these two arguments are different (i.e., Line 1129).  I could pass the FPMathFlags in, but then I would be hoisting the type checking out of the static helper functions, which would defeat the purpose.

================
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())
----------------
Chandler Carruth wrote:
> Why not floating point vector instructions? That seems rather odd to group with testing for fastmath...
I've already addressed this internally, but I need to rerun correctness and performance.

http://reviews.llvm.org/D4129