[opaque pointer types] Cleanup CGBuilder's Create*GEP.

Some of these functions take some extraneous arguments, e.g. EltSize,
Offset, which are computable from the Type and DataLayout.

Add some asserts to ensure that the computed values are consistent
with the passed-in values, in preparation for eliminating the
extraneous arguments. This also asserts that the Type is an Array for
the calls named "Array" and a Struct for the calls named "Struct".

Then, correct a couple of errors:

1. Using CreateStructGEP on an array type. (this causes the majority
   of the test differences, as struct GEPs are created with i32
   indices, while array GEPs are created with i64 indices)

2. Passing the wrong Offset to CreateStructGEP in TargetInfo.cpp on
   x86-64 NACL (which uses 32-bit pointers).

Differential Revision: https://reviews.llvm.org/D57766

llvm-svn: 353529

4 files changed, 56 insertions, 7 deletions

diff --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index b614e255296..75280002279 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -170,6 +170,13 @@ public:
   using CGBuilderBaseTy::CreateStructGEP;
   Address CreateStructGEP(Address Addr, unsigned Index, CharUnits Offset,
                           const llvm::Twine &Name = "") {
+#ifndef NDEBUG
+    llvm::StructType *ElTy = cast<llvm::StructType>(Addr.getElementType());
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    const llvm::StructLayout *SL = DL.getStructLayout(ElTy);
+    assert(SL->getElementOffset(Index) == (uint64_t)Offset.getQuantity());
+#endif
+
     return Address(CreateStructGEP(Addr.getElementType(),
                                    Addr.getPointer(), Index, Name),
                    Addr.getAlignment().alignmentAtOffset(Offset));
@@ -177,6 +184,13 @@ public:
   Address CreateStructGEP(Address Addr, unsigned Index,
                           const llvm::StructLayout *Layout,
                           const llvm::Twine &Name = "") {
+#ifndef NDEBUG
+    llvm::StructType *ElTy = cast<llvm::StructType>(Addr.getElementType());
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    const llvm::StructLayout *SL = DL.getStructLayout(ElTy);
+    assert(Layout == SL);
+#endif
+
     auto Offset = CharUnits::fromQuantity(Layout->getElementOffset(Index));
     return CreateStructGEP(Addr, Index, Offset, Name);
   }
@@ -193,6 +207,13 @@ public:
   /// \param EltSize - the size of the type T in bytes
   Address CreateConstArrayGEP(Address Addr, uint64_t Index, CharUnits EltSize,
                               const llvm::Twine &Name = "") {
+#ifndef NDEBUG
+    llvm::ArrayType *ElTy = cast<llvm::ArrayType>(Addr.getElementType());
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    assert(DL.getTypeAllocSize(ElTy->getElementType()) ==
+           (uint64_t)EltSize.getQuantity());
+#endif
+
     return Address(CreateInBoundsGEP(Addr.getPointer(),
                                      {getSize(CharUnits::Zero()),
                                       getSize(Index)},
@@ -200,6 +221,19 @@ public:
                    Addr.getAlignment().alignmentAtOffset(Index * EltSize));
   }
 
+  Address CreateConstArrayGEP(Address Addr, uint64_t Index,
+                              const llvm::Twine &Name = "") {
+    llvm::ArrayType *ElTy = cast<llvm::ArrayType>(Addr.getElementType());
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    CharUnits EltSize =
+        CharUnits::fromQuantity(DL.getTypeAllocSize(ElTy->getElementType()));
+
+    return Address(
+        CreateInBoundsGEP(Addr.getPointer(),
+                          {getSize(CharUnits::Zero()), getSize(Index)}, Name),
+        Addr.getAlignment().alignmentAtOffset(Index * EltSize));
+  }
+
   /// Given
   ///   %addr = T* ...
   /// produce
@@ -210,6 +244,12 @@ public:
   Address CreateConstInBoundsGEP(Address Addr, uint64_t Index,
                                  CharUnits EltSize,
                                  const llvm::Twine &Name = "") {
+#ifndef NDEBUG
+    llvm::Type *ElTy = Addr.getElementType();
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    assert(DL.getTypeAllocSize(ElTy) == (uint64_t)EltSize.getQuantity());
+#endif
+
     return Address(CreateInBoundsGEP(Addr.getElementType(), Addr.getPointer(),
                                      getSize(Index), Name),
                    Addr.getAlignment().alignmentAtOffset(Index * EltSize));
@@ -224,6 +264,11 @@ public:
   /// \param EltSize - the size of the type T in bytes
   Address CreateConstGEP(Address Addr, uint64_t Index, CharUnits EltSize,
                          const llvm::Twine &Name = "") {
+#ifndef NDEBUG
+    const llvm::DataLayout &DL = BB->getParent()->getParent()->getDataLayout();
+    assert(DL.getTypeAllocSize(Addr.getElementType()) ==
+           (uint64_t)EltSize.getQuantity());
+#endif
     return Address(CreateGEP(Addr.getElementType(), Addr.getPointer(),
                              getSize(Index), Name),
                    Addr.getAlignment().alignmentAtOffset(Index * EltSize));
@@ -247,6 +292,11 @@ public:
   Address CreateConstInBoundsGEP2_32(Address Addr, unsigned Idx0,
                                       unsigned Idx1, const llvm::DataLayout &DL,
                                       const llvm::Twine &Name = "") {
+#ifndef NDEBUG
+    const llvm::DataLayout &DL2 = BB->getParent()->getParent()->getDataLayout();
+    assert(DL == DL2);
+#endif
+
     auto *GEP = cast<llvm::GetElementPtrInst>(CreateConstInBoundsGEP2_32(
         Addr.getElementType(), Addr.getPointer(), Idx0, Idx1, Name));
     llvm::APInt Offset(
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index d8eaad1a49f..e0ebf842241 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3258,7 +3258,7 @@ Address CodeGenFunction::EmitArrayToPointerDecay(const Expr *E,
   if (!E->getType()->isVariableArrayType()) {
     assert(isa<llvm::ArrayType>(Addr.getElementType()) &&
            "Expected pointer to array");
-    Addr = Builder.CreateStructGEP(Addr, 0, CharUnits::Zero(), "arraydecay");
+    Addr = Builder.CreateConstArrayGEP(Addr, 0, "arraydecay");
   }
 
   // The result of this decay conversion points to an array element within the
@@ -3535,8 +3535,7 @@ static Address emitOMPArraySectionBase(CodeGenFunction &CGF, const Expr *Base,
       if (!BaseTy->isVariableArrayType()) {
         assert(isa<llvm::ArrayType>(Addr.getElementType()) &&
                "Expected pointer to array");
-        Addr = CGF.Builder.CreateStructGEP(Addr, 0, CharUnits::Zero(),
-                                           "arraydecay");
+        Addr = CGF.Builder.CreateConstArrayGEP(Addr, 0, "arraydecay");
       }
 
       return CGF.Builder.CreateElementBitCast(Addr,
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 5fb5b648841..deb81ebd650 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5169,8 +5169,7 @@ void CGOpenMPRuntime::emitTaskCall(CodeGenFunction &CGF, SourceLocation Loc,
                             FlagsLVal);
     }
     DependenciesArray = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
-        CGF.Builder.CreateStructGEP(DependenciesArray, 0, CharUnits::Zero()),
-        CGF.VoidPtrTy);
+        CGF.Builder.CreateConstArrayGEP(DependenciesArray, 0), CGF.VoidPtrTy);
   }
 
   // NOTE: routine and part_id fields are initialized by __kmpc_omp_task_alloc()
diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index fa5b6fdff77..d052e5719a0 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -3753,8 +3753,9 @@ Address X86_64ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
   // loads than necessary. Can we clean this up?
   llvm::Type *LTy = CGF.ConvertTypeForMem(Ty);
   llvm::Value *RegSaveArea = CGF.Builder.CreateLoad(
-      CGF.Builder.CreateStructGEP(VAListAddr, 3, CharUnits::fromQuantity(16)),
-                                  "reg_save_area");
+      CGF.Builder.CreateStructGEP(
+          VAListAddr, 3, CharUnits::fromQuantity(8) + CGF.getPointerSize()),
+      "reg_save_area");
 
   Address RegAddr = Address::invalid();
   if (neededInt && neededSSE) {