[PATCH] IR: Do not canonicalize constant GEPs into an out-of-bounds array access

[Chandler Carruth]

  Sorry for the late comments. I wrote these and forgot to mash "send". They're all tiny nit-picks, so feel free to address them whenever in subsequent commits.


================
Comment at: lib/IR/ConstantFold.cpp:1979
@@ -1944,1 +1978,3 @@
+      }
+      if (IsSequentialAccessInRange || Idx0->isNullValue()) {
         SmallVector<Value*, 16> NewIndices;
----------------
If we know Idx0->isNullValue() do we really want to compute IsSequentialAccessInRange? It looks reasonably expensive.

================
Comment at: lib/IR/ConstantFold.cpp:1961
@@ +1960,3 @@
+      // if the index is in-range with respect to an array or vector.
+      bool IsSequentialAccessInRange = false;
+      if (LastTy && isa<SequentialType>(LastTy)) {
----------------
I would just hoist computing this into a static helper function. That will make everything flow better.

================
Comment at: lib/IR/ConstantFold.cpp:1974
@@ +1973,3 @@
+            IsSequentialAccessInRange = true;
+        } else if (PointerType *PTy = dyn_cast<PointerType>(LastTy))
+          // Only handle pointers to sized types, not pointers to functions.
----------------
When the if needs {}s, I prefer to also put them on any else attached. Especially an else with multiple lines even if only one is a statement. Especially if the one statement is itself multiple lines because it is an if statement. =]

================
Comment at: lib/IR/ConstantFold.cpp:1963-1967
@@ +1962,7 @@
+      if (LastTy && isa<SequentialType>(LastTy)) {
+        uint64_t NumElements = 0;
+        if (ArrayType *ATy = dyn_cast<ArrayType>(LastTy))
+          NumElements = ATy->getNumElements();
+        else if (VectorType *VTy = dyn_cast<VectorType>(LastTy))
+          NumElements = VTy->getNumElements();
+
----------------
Sink this into the branch where we have a constant int index.


http://llvm-reviews.chandlerc.com/D2093

COMMIT
  http://llvm-reviews.chandlerc.com/rL194220