[PATCH] D16302: [opaque pointer types] [NFC] ConstantFoldInstOperands: add a form taking the Instruction or ConstantExpr for GEPs.

[Manuel Jacob via llvm-commits]

mjacob added a comment.

I'm stopping reviewing here. Please create a separate patch for the changes in `SymbolicallyEvaluateGEP`.


================
Comment at: include/llvm/Analysis/ConstantFolding.h:50-55
@@ -48,2 +49,8 @@
 
+  Constant *ConstantFoldInstOperands(const Value *InstOrCE,
+                                     unsigned Opcode, Type *DestTy,
+                                     ArrayRef<Constant *> Ops,
+                                     const DataLayout &DL,
+                                     const TargetLibraryInfo *TLI = nullptr);
+
 /// ConstantFoldInstOperands - Attempt to constant fold an instruction with the
----------------
Instead of introducing this function with the implicit requirement of having to provide `InstOrCE` if `Opcode == Instruction::GetElementPtr`, this should be made explicit by introducing a function specifically for GEPs (similar to `ConstantFoldCompareInstOperands()`).

================
Comment at: lib/Analysis/ConstantFolding.cpp:729-730
@@ -729,3 +728,4 @@
 /// If we can symbolically evaluate the GEP constant expression, do so.
 static Constant *SymbolicallyEvaluateGEP(Type *SrcTy, ArrayRef<Constant *> Ops,
-                                         Type *ResultTy, const DataLayout &DL,
+                                         Type *ResultTy, Type *ResultElementTy,
+                                         const DataLayout &DL,
----------------
The lack of consistency of parameter names is slightly confusing here. Am I right that `ResultTy` is the pointer type of the GEP, and `SrcTy` and `ResultElementTy` are pointee types?

================
Comment at: lib/Analysis/ConstantFolding.cpp:734
@@ -733,4 +733,3 @@
   Constant *Ptr = Ops[0];
-  if (!Ptr->getType()->getPointerElementType()->isSized() ||
-      !Ptr->getType()->isPointerTy())
+  if (!SrcTy->isSized() || !Ptr->getType()->isPointerTy())
     return nullptr;
----------------
`Ptr` is always a constant of pointer type, right? I'll remove this check in a separate commit.


http://reviews.llvm.org/D16302