From 45a441667e7c26607b2ff20dea66f68d2435a6da Mon Sep 17 00:00:00 2001
From: Arina Neshlyaeva <arina.neshlyaeva@intel.com>
Date: Mon, 19 Sep 2022 09:46:16 -0700
Subject: [PATCH] Eliminate usage of getPointerElementType in GEP

---
 src/ctx.cpp                         | 21 ++++++++++++++-------
 src/ctx.h                           |  3 +++
 src/expr.cpp                        | 27 ++++++++++++++-------------
 tests/lit-tests/load_unaligned.ispc |  9 +++++++++
 4 files changed, 40 insertions(+), 20 deletions(-)
 create mode 100644 tests/lit-tests/load_unaligned.ispc

diff --git a/src/ctx.cpp b/src/ctx.cpp
index 9480f8567..229b9cf4f 100644
--- a/src/ctx.cpp
+++ b/src/ctx.cpp
@@ -240,6 +240,12 @@ AddressInfo::AddressInfo(llvm::Value *p, const Type *t) : pointer(p), ispcType(t
     Assert(elementType != nullptr && "Element type cannot be null");
 }
 
+llvm::Type *AddressInfo::GetPointeeLLVMType(const PointerType *pt) {
+    Assert(pt != nullptr && "ISPC type cannot be null");
+    llvm::Type *type = pt->GetBaseType()->LLVMStorageType(g->ctx);
+    Assert(type != nullptr && "LLVM pointer element type cannot be null");
+    return type;
+}
 ///////////////////////////////////////////////////////////////////////////
 
 FunctionEmitContext::FunctionEmitContext(Function *func, Symbol *funSym, llvm::Function *lf, SourcePos firstStmtPos) {
@@ -2103,7 +2109,8 @@ llvm::Value *FunctionEmitContext::GetElementPtrInst(llvm::Value *basePtr, llvm::
         // uniform, so just emit the regular LLVM GEP instruction
         llvm::Value *ind[1] = {index};
         llvm::ArrayRef<llvm::Value *> arrayRef(&ind[0], &ind[1]);
-        llvm::Instruction *inst = llvm::GetElementPtrInst::Create(PTYPE(basePtr), basePtr, arrayRef,
+        llvm::Type *llvmPtrElType = AddressInfo::GetPointeeLLVMType(ptrType);
+        llvm::Instruction *inst = llvm::GetElementPtrInst::Create(llvmPtrElType, basePtr, arrayRef,
                                                                   name.isTriviallyEmpty() ? "gep" : name, bblock);
         AddDebugPos(inst);
         return inst;
@@ -2146,7 +2153,8 @@ llvm::Value *FunctionEmitContext::GetElementPtrInst(llvm::Value *basePtr, llvm::
         // uniform, so just emit the regular LLVM GEP instruction
         llvm::Value *indices[2] = {index0, index1};
         llvm::ArrayRef<llvm::Value *> arrayRef(&indices[0], &indices[2]);
-        llvm::Instruction *inst = llvm::GetElementPtrInst::Create(PTYPE(basePtr), basePtr, arrayRef,
+        llvm::Type *llvmPtrElType = AddressInfo::GetPointeeLLVMType(ptrType);
+        llvm::Instruction *inst = llvm::GetElementPtrInst::Create(llvmPtrElType, basePtr, arrayRef,
                                                                   name.isTriviallyEmpty() ? "gep" : name, bblock);
         AddDebugPos(inst);
         return inst;
@@ -2429,11 +2437,10 @@ llvm::Value *FunctionEmitContext::LoadInst(llvm::Value *ptr, llvm::Value *mask,
                 new llvm::LoadInst(ptr, name.isTriviallyEmpty() ? (llvm::Twine(ptr->getName()) + "_load") : name,
                                    false /* not volatile */, bblock);
 #else // LLVM 11.0+
-            llvm::PointerType *ptr_type = llvm::dyn_cast<llvm::PointerType>(ptr->getType());
-            llvm::LoadInst *inst =
-                new llvm::LoadInst(ptr_type->getPointerElementType(), ptr,
-                                   name.isTriviallyEmpty() ? (llvm::Twine(ptr->getName()) + "_load") : name,
-                                   false /* not volatile */, bblock);
+            llvm::Type *llvmPtrType = AddressInfo::GetPointeeLLVMType(ptrType);
+            llvm::LoadInst *inst = new llvm::LoadInst(
+                llvmPtrType, ptr, name.isTriviallyEmpty() ? (llvm::Twine(ptr->getName()) + "_load") : name,
+                false /* not volatile */, bblock);
 #endif
 
             if (atomicType != NULL && atomicType->IsVaryingType()) {
diff --git a/src/ctx.h b/src/ctx.h
index cc9fe9350..1a7375dc9 100644
--- a/src/ctx.h
+++ b/src/ctx.h
@@ -70,6 +70,9 @@ class AddressInfo {
     // Return the address space that this address resides in.
     unsigned getAddressSpace() const { return getType()->getAddressSpace(); }
 
+    // Get LLVM pointer element type from ISPC PointerType.
+    static llvm::Type *GetPointeeLLVMType(const PointerType *pt);
+
   private:
     llvm::Value *pointer;
     llvm::Type *elementType;
diff --git a/src/expr.cpp b/src/expr.cpp
index 2e5d44a7c..de10c2cda 100644
--- a/src/expr.cpp
+++ b/src/expr.cpp
@@ -760,9 +760,11 @@ void ispc::InitSymbol(AddressInfo *ptrInfo, const Type *symType, Expr *initExpr,
                 llvm::Value *ep;
                 if (CastType<StructType>(symType) != NULL)
                     ep = ctx->AddElementOffset(ptrInfo->getPointer(), i, NULL, "element");
-                else
+                else {
                     ep = ctx->GetElementPtrInst(ptrInfo->getPointer(), LLVMInt32(0), LLVMInt32(i),
-                                                PointerType::GetUniform(elementType), "gep");
+                                                /* Type of aggregate structure */ PointerType::GetUniform(symType),
+                                                "gep");
+                }
                 AddressInfo *epInfo = new AddressInfo(ep, ptrInfo->getElementType());
                 if (i < nInits)
                     InitSymbol(epInfo, elementType, exprList->exprs[i], ctx, pos);
@@ -2892,7 +2894,7 @@ static std::pair<llvm::Constant *, bool> lGetBinaryExprStorageConstant(const Typ
         isNotValidForMultiTargetGlobal = isNotValidForMultiTargetGlobal || c2Pair.second;
         if (op == BinaryExpr::Op::Sub)
             c2 = llvm::ConstantExpr::getNeg(c2);
-        llvm::Constant *c = llvm::ConstantExpr::getGetElementPtr(PTYPE(c1), c1, c2);
+        llvm::Constant *c = llvm::ConstantExpr::getGetElementPtr(AddressInfo::GetPointeeLLVMType(pt0), c1, c2);
         return std::pair<llvm::Constant *, bool>(c, isNotValidForMultiTargetGlobal);
     } else if (const PointerType *pt1 = CastType<PointerType>(arg1->GetType())) {
         std::pair<llvm::Constant *, bool> c1Pair;
@@ -2912,7 +2914,7 @@ static std::pair<llvm::Constant *, bool> lGetBinaryExprStorageConstant(const Typ
             c2Pair = cExpr->GetConstant(cExpr->GetType());
         llvm::Constant *c2 = c2Pair.first;
         isNotValidForMultiTargetGlobal = isNotValidForMultiTargetGlobal || c2Pair.second;
-        llvm::Constant *c = llvm::ConstantExpr::getGetElementPtr(PTYPE(c1), c1, c2);
+        llvm::Constant *c = llvm::ConstantExpr::getGetElementPtr(AddressInfo::GetPointeeLLVMType(pt1), c1, c2);
         return std::pair<llvm::Constant *, bool>(c, isNotValidForMultiTargetGlobal);
     }
 
@@ -5073,13 +5075,10 @@ llvm::Value *VectorMemberExpr::GetValue(FunctionEmitContext *ctx) const {
         // to the same logic where it's used elsewhere
         llvm::Value *elementMask = ctx->GetFullMask();
 
-        const Type *elementPtrType = NULL;
-        if (CastType<ReferenceType>(basePtrType) != NULL)
-            elementPtrType = PointerType::GetUniform(basePtrType->GetReferenceTarget());
-        else
-            elementPtrType = basePtrType->IsUniformType() ? PointerType::GetUniform(exprVectorType->GetElementType())
-                                                          : PointerType::GetVarying(exprVectorType->GetElementType());
-
+        const PointerType *ptrType = ctx->RegularizePointer(basePtrType);
+        const Type *elementPtrType = ptrType->IsUniformType()
+                                         ? PointerType::GetUniform(exprVectorType->GetElementType())
+                                         : PointerType::GetVarying(exprVectorType->GetElementType());
         ctx->SetDebugPos(pos);
         for (size_t i = 0; i < identifier.size(); ++i) {
             char idStr[2] = {identifier[i], '\0'};
@@ -7221,7 +7220,8 @@ std::pair<llvm::Constant *, bool> TypeCastExpr::GetConstant(const Type *constTyp
             if (llvm::Constant *c = llvm::dyn_cast<llvm::Constant>(ptr)) {
                 llvm::Value *offsets[2] = {LLVMInt32(0), LLVMInt32(0)};
                 llvm::ArrayRef<llvm::Value *> arrayRef(&offsets[0], &offsets[2]);
-                llvm::Value *resultPtr = llvm::ConstantExpr::getGetElementPtr(PTYPE(c), c, arrayRef);
+                llvm::Value *resultPtr = llvm::ConstantExpr::getGetElementPtr(
+                    llvm::dyn_cast<llvm::GlobalVariable>(ptr)->getValueType(), c, arrayRef);
                 if (resultPtr->getType() == constType->LLVMType(g->ctx)) {
                     llvm::Constant *ret = llvm::dyn_cast<llvm::Constant>(resultPtr);
                     return std::pair<llvm::Constant *, bool>(ret, false);
@@ -7630,7 +7630,8 @@ std::pair<llvm::Constant *, bool> AddressOfExpr::GetConstant(const Type *type) c
                     return std::pair<llvm::Constant *, bool>(NULL, false);
                 gepIndex.insert(gepIndex.begin(), LLVMInt64(0));
                 llvm::Constant *c = llvm::cast<llvm::Constant>(ptr);
-                llvm::Constant *c1 = llvm::ConstantExpr::getGetElementPtr(PTYPE(c), c, gepIndex);
+                llvm::Constant *c1 = llvm::ConstantExpr::getGetElementPtr(
+                    llvm::dyn_cast<llvm::GlobalVariable>(ptr)->getValueType(), c, gepIndex);
                 return std::pair<llvm::Constant *, bool>(c1, isNotValidForMultiTargetGlobal);
             }
         }
diff --git a/tests/lit-tests/load_unaligned.ispc b/tests/lit-tests/load_unaligned.ispc
new file mode 100644
index 000000000..3b4431c04
--- /dev/null
+++ b/tests/lit-tests/load_unaligned.ispc
@@ -0,0 +1,9 @@
+// This test checks reduced alignment for varying atomic types
+// RUN: %{ispc} %s --nostdlib --emit-llvm-text -o - | FileCheck %s
+typedef float<3> float3;
+// CHECK-COUNT-3: load <{{[0-9]+}} x float>, <{{[0-9]+}} x float>* %{{.*}}, align 1
+
+extern float3 TestLoadUnaligned(float3 &b) {
+    float3 xyz = b.xyz;
+    return xyz;
+}
