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

[Chandler Carruth]

Sorry I kept failing to get back to this.

Feel free to submit whenever. See comments below for my only (minor) comments. I don't see anything needing to wait on more rounds of review though.

I look forward to seeing the next 3 things I see here:

1) Support swapping the operand order once SLP vectorizer is OK with it
2) Support for floating point vectors
3) (maybe?) Support for integer vectors?

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:884
@@ +883,3 @@
+      } else
+        ExpressionChanged->clearSubclassOptionalData();
+
----------------
Why not do this before or ofter the if, but outside of the if? It's duplicated in both sides.

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:1965-1968
@@ -1847,3 +1964,6 @@
 
-    return;
+    // Don't try to optimize vector instructions or anything that doesn't have
+    // unsafe algebra.
+    if (I->getType()->isVectorTy() || !I->hasUnsafeAlgebra())
+      return;
   }
----------------
I would really, really like to know why on earth we don't reassociate integer vector operations. That seems... kind of crazy.

================
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);
 }
----------------
Chad Rosier wrote:
> 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.
I see. This makes sense. On looking at things more carefully, I actually like the pattern you use here more than the pattern for CreateMul and CreateAdd. It seems strange to conflate "insertion point" and "fast math flags source". Is there some conceptual reason to have them be the same? If not, I'd split them in all the methods.

http://reviews.llvm.org/D4129