[PATCH] [NaryReassoc] reassociate GEP for CSE

[Bjarke Hammersholt Roune]

================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:402
@@ +401,3 @@
+  if (RHS->getType() != IntPtrTy)
+    RHS = Builder.CreateSExt(RHS, IntPtrTy);
+  if (IndexedSize != ElementSize) {
----------------
Suppose that RHS is 64 bits and pointers are 32 bits (or if RHS is >64 bits), do we then need CreateSExtOrTrunc here instead of CreateSExt?

================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:264
@@ +263,3 @@
+  default:
+    llvm_unreachable("should be filtered out by isPotentiallyNaryReassociable");
+  }
----------------
jingyue wrote:
> broune wrote:
> > What's the motivation behind doing it this way instead of returning null here and then not needing isPotentiallyNaryReassociable?
> If `!isPotentiallyNaryReassociable`, we don't need to add `I` to `SeenExprs`. I could merge `SeenExprs[SE->getSCEV(I)].push_back(I)` to `tryReassociate`; then `tryReassociate` should probably be renamed to `tryReassociateAndAddCandidate`? 
I'm fine with it as it is now. Merging it into tryReassociate seems fine too.

http://reviews.llvm.org/D9802

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/