[PATCH] CSE on GEP indices

[Jingyue Wu]

All comments addressed. 

Hal and Justin, do you have more to add? Looking forward to your feedback.

================
Comment at: lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp:169
@@ +168,3 @@
+
+  /// The path from the GEP index to the constant offset.
+  SmallVector<User *, 8> UserChain;
----------------
Eli Bendersky wrote:
> It would be nice to document the structure of UserChain. E.g. [0] is the discovered constant, etc. A simple example could also be useful here. 
done

================
Comment at: lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp:341
@@ +340,3 @@
+  unsigned OpNo = FindFirstUse(U, C); // U->getOperand(OpNo) == C
+  assert(OpNo != (unsigned)-1 && OpNo < 2 &&
+         "UserChain wasn't built correctly");
----------------
Eli Bendersky wrote:
> OpNo < 2 covers you, you don't need != (unsigned)-1 too.
done

================
Comment at: lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp:356
@@ +355,3 @@
+  // clone of the subexpression but with the constant removed.
+  // e.g., to build a clone of (a + (b + 5)) but with the 5 removed, we first
+  // build 0, then (b + 0), and finally (a + (b + 0)).
----------------
Eli Bendersky wrote:
> I find this comment a bit confusing when looking at rebuildLeafWithoutConstantOffset, which doesn't insert a 0 but just leaves the constant out.
The comment was still describing an old implementation. good catch. Thanks! 

================
Comment at: test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll:61
@@ +60,3 @@
+
+; We should treat "or" with no common bits as "add".
+define float* @or(i64 %i) {
----------------
Eli Bendersky wrote:
> I'd also add a test where there are common bits, to make sure we don't cause a miscompilation.
Modified function @or to cover this case

http://reviews.llvm.org/D3462