Fix corner cases for pointer offset codegen

Fixes ldc-developers#4362.
kinke · Apr 4, 2023 · 199a549 · 199a549
1 parent 0d4d711
commit 199a549
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 35 deletions.
diff --git a/gen/binops.cpp b/gen/binops.cpp
@@ -21,17 +21,6 @@
 
 //////////////////////////////////////////////////////////////////////////////
 
-dinteger_t undoStrideMul(const Loc &loc, Type *t, dinteger_t offset) {
-  assert(t->ty == TY::Tpointer);
-  const auto elemSize = t->nextOf()->size(loc);
-  assert((offset % elemSize) == 0 &&
-         "Expected offset by an integer amount of elements");
-
-  return offset / elemSize;
-}
-
-//////////////////////////////////////////////////////////////////////////////
-
 namespace {
 struct RVals {
   DRValue *lhs, *rhs;
@@ -89,30 +78,37 @@ DValue *emitPointerOffset(Loc loc, DValue *base, Expression *offset,
   // pointer elements. We try to undo this before resorting to
   // temporarily bitcasting the pointer to i8.
 
-  LLType * llBaseTy = nullptr;
+  Type* const pointeeType = base->type->nextOf();
+
+  LLType * llBaseTy = DtoMemType(pointeeType);
   LLValue *llBase = nullptr;
   LLValue *llOffset = nullptr;
   LLValue *llResult = nullptr;
 
   if (offset->isConst()) {
     llBase = DtoRVal(base);
-    llBaseTy = DtoMemType(base->type->nextOf());
-    dinteger_t byteOffset = offset->toInteger();
+    const dinteger_t byteOffset = offset->toInteger();
     if (byteOffset == 0) {
       llResult = llBase;
     } else {
-      llOffset = DtoConstSize_t(undoStrideMul(loc, base->type, byteOffset));
+      const auto pointeeSize = pointeeType->size(loc);
+      if (pointeeSize && byteOffset % pointeeSize == 0) { // can do a nice GEP
+        llOffset = DtoConstSize_t(byteOffset / pointeeSize);
+      } else { // need to cast base to i8*
+        llBaseTy = getI8Type();
+        llBase = DtoBitCast(llBase, getVoidPtrType());
+        llOffset = DtoConstSize_t(byteOffset);
+      }
     }
   } else {
-    Expression *noStrideInc = extractNoStrideInc(
-        offset, base->type->nextOf()->size(loc), negateOffset);
+    Expression *noStrideInc =
+        extractNoStrideInc(offset, pointeeType->size(loc), negateOffset);
     auto rvals =
         evalSides(base, noStrideInc ? noStrideInc : offset, loadLhsAfterRhs);
     llBase = DtoRVal(rvals.lhs);
-    llBaseTy = DtoMemType(rvals.lhs->type->nextOf());
     llOffset = DtoRVal(rvals.rhs);
     if (!noStrideInc) { // byte offset => cast base to i8*
-      llBaseTy = LLType::getInt8Ty(gIR->context());
+      llBaseTy = getI8Type();
       llBase = DtoBitCast(llBase, getVoidPtrType());
     }
   }

diff --git a/gen/binops.h b/gen/binops.h
@@ -60,5 +60,3 @@ llvm::Value *DtoBinNumericEquals(const Loc &loc, DValue *lhs, DValue *rhs,
 llvm::Value *DtoBinFloatsEquals(const Loc &loc, DValue *lhs, DValue *rhs,
                                 EXP op);
 llvm::Value *mergeVectorEquals(llvm::Value *resultsVector, EXP op);
-
-dinteger_t undoStrideMul(const Loc &loc, Type *t, dinteger_t offset);
diff --git a/gen/toconstelem.cpp b/gen/toconstelem.cpp
@@ -187,18 +187,45 @@ class ToConstElemVisitor : public Visitor {
 
   //////////////////////////////////////////////////////////////////////////////
 
+  // very similar to emitPointerOffset() in binops.cpp
+  LLConstant *tryEmitPointerOffset(BinExp *e, bool negateOffset) {
+    Type *t1b = e->e1->type->toBasetype();
+    if (t1b->ty != TY::Tpointer || !e->e2->type->isintegral())
+      return nullptr;
+
+    Type *const pointeeType = t1b->nextOf();
+
+    LLConstant *llBase = toConstElem(e->e1, p);
+    const dinteger_t byteOffset = e->e2->toInteger();
+
+    LLConstant *llResult = nullptr;
+    const auto pointeeSize = pointeeType->size(e->loc);
+    if (pointeeSize && byteOffset % pointeeSize == 0) { // can do a nice GEP
+      LLConstant *llOffset = DtoConstSize_t(byteOffset / pointeeSize);
+      if (negateOffset)
+        llOffset = llvm::ConstantExpr::getNeg(llOffset);
+      llResult = llvm::ConstantExpr::getGetElementPtr(DtoMemType(pointeeType),
+                                                      llBase, llOffset);
+    } else { // need to cast base to i8*
+      llBase = DtoBitCast(llBase, getVoidPtrType());
+      LLConstant *llOffset = DtoConstSize_t(byteOffset);
+      if (negateOffset)
+        llOffset = llvm::ConstantExpr::getNeg(llOffset);
+      llResult =
+          llvm::ConstantExpr::getGetElementPtr(getI8Type(), llBase, llOffset);
+    }
+
+    return DtoBitCast(llResult, DtoType(e->type));
+  }
+
   void visit(AddExp *e) override {
     IF_LOG Logger::print("AddExp::toConstElem: %s @ %s\n", e->toChars(),
                          e->type->toChars());
     LOG_SCOPE;
 
     // add to pointer
-    Type *t1b = e->e1->type->toBasetype();
-    if (t1b->ty == TY::Tpointer && e->e2->type->isintegral()) {
-      llvm::Constant *ptr = toConstElem(e->e1, p);
-      dinteger_t idx = undoStrideMul(e->loc, t1b, e->e2->toInteger());
-      result = llvm::ConstantExpr::getGetElementPtr(DtoType(e->e1->type), ptr,
-                                                    DtoConstSize_t(idx));
+    if (auto r = tryEmitPointerOffset(e, false)) {
+      result = r;
       return;
     }
 
@@ -210,14 +237,9 @@ class ToConstElemVisitor : public Visitor {
                          e->type->toChars());
     LOG_SCOPE;
 
-    Type *t1b = e->e1->type->toBasetype();
-    if (t1b->ty == TY::Tpointer && e->e2->type->isintegral()) {
-      llvm::Constant *ptr = toConstElem(e->e1, p);
-      dinteger_t idx = undoStrideMul(e->loc, t1b, e->e2->toInteger());
-
-      llvm::Constant *negIdx = llvm::ConstantExpr::getNeg(DtoConstSize_t(idx));
-      result = llvm::ConstantExpr::getGetElementPtr(DtoType(e->e1->type), ptr,
-                                                    negIdx);
+    // subtract from pointer
+    if (auto r = tryEmitPointerOffset(e, true)) {
+      result = r;
       return;
     }
 

diff --git a/tests/codegen/pointer_offsets_gh4362.d b/tests/codegen/pointer_offsets_gh4362.d
@@ -0,0 +1,9 @@
+// RUN: %ldc -run %s
+
+void main()
+{
+    int[2][1] arr;
+    assert(&(arr[0][0]) !is &(arr[0][1]));
+    ubyte RR = 0;
+    assert(&(arr[RR][0]) !is &(arr[RR][1])); // fails
+}