[llvm] 02dda1c - [Local] Clean up EmitGEPOffset

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 13 09:31:06 PST 2020


Author: Nikita Popov
Date: 2020-11-13T18:30:56+01:00
New Revision: 02dda1c659e8d675a20ace06dc060b787345b972

URL: https://github.com/llvm/llvm-project/commit/02dda1c659e8d675a20ace06dc060b787345b972
DIFF: https://github.com/llvm/llvm-project/commit/02dda1c659e8d675a20ace06dc060b787345b972.diff

LOG: [Local] Clean up EmitGEPOffset

Handle the emission of the add in a single place, instead of three
different ones.

Don't emit an unnecessary add with zero to start with. It will get
dropped by InstCombine, but we may as well not create it in the
first place. This also means that InstCombine does not need to
specially handle this extra add.

This is conceptually NFC, but can affect worklist order etc.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/Utils/Local.h
    llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/test/Transforms/InstCombine/sub-gep.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/Utils/Local.h b/llvm/include/llvm/Analysis/Utils/Local.h
index 9da0c6586dac..f8892c85e145 100644
--- a/llvm/include/llvm/Analysis/Utils/Local.h
+++ b/llvm/include/llvm/Analysis/Utils/Local.h
@@ -30,7 +30,7 @@ Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout &DL, User *GEP,
                      bool NoAssumptions = false) {
   GEPOperator *GEPOp = cast<GEPOperator>(GEP);
   Type *IntIdxTy = DL.getIndexType(GEP->getType());
-  Value *Result = Constant::getNullValue(IntIdxTy);
+  Value *Result = nullptr;
 
   // If the GEP is inbounds, we know that none of the addressing operations will
   // overflow in a signed sense.
@@ -46,6 +46,7 @@ Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout &DL, User *GEP,
        ++i, ++GTI) {
     Value *Op = *i;
     uint64_t Size = DL.getTypeAllocSize(GTI.getIndexedType()) & PtrSizeMask;
+    Value *Offset;
     if (Constant *OpC = dyn_cast<Constant>(Op)) {
       if (OpC->isZeroValue())
         continue;
@@ -54,46 +55,46 @@ Value *EmitGEPOffset(IRBuilderTy *Builder, const DataLayout &DL, User *GEP,
       if (StructType *STy = GTI.getStructTypeOrNull()) {
         uint64_t OpValue = OpC->getUniqueInteger().getZExtValue();
         Size = DL.getStructLayout(STy)->getElementOffset(OpValue);
-
-        if (Size)
-          Result = Builder->CreateAdd(Result, ConstantInt::get(IntIdxTy, Size),
-                                      GEP->getName().str()+".offs");
-        continue;
+        if (!Size)
+          continue;
+
+        Offset = ConstantInt::get(IntIdxTy, Size);
+      } else {
+        // Splat the constant if needed.
+        if (IntIdxTy->isVectorTy() && !OpC->getType()->isVectorTy())
+          OpC = ConstantVector::getSplat(
+              cast<VectorType>(IntIdxTy)->getElementCount(), OpC);
+
+        Constant *Scale = ConstantInt::get(IntIdxTy, Size);
+        Constant *OC =
+            ConstantExpr::getIntegerCast(OpC, IntIdxTy, true /*SExt*/);
+        Offset =
+            ConstantExpr::getMul(OC, Scale, false /*NUW*/, isInBounds /*NSW*/);
       }
-
-      // Splat the constant if needed.
-      if (IntIdxTy->isVectorTy() && !OpC->getType()->isVectorTy())
-        OpC = ConstantVector::getSplat(
-            cast<VectorType>(IntIdxTy)->getElementCount(), OpC);
-
-      Constant *Scale = ConstantInt::get(IntIdxTy, Size);
-      Constant *OC = ConstantExpr::getIntegerCast(OpC, IntIdxTy, true /*SExt*/);
-      Scale =
-          ConstantExpr::getMul(OC, Scale, false /*NUW*/, isInBounds /*NSW*/);
-      // Emit an add instruction.
-      Result = Builder->CreateAdd(Result, Scale, GEP->getName().str()+".offs");
-      continue;
-    }
-
-    // Splat the index if needed.
-    if (IntIdxTy->isVectorTy() && !Op->getType()->isVectorTy())
-      Op = Builder->CreateVectorSplat(
-          cast<FixedVectorType>(IntIdxTy)->getNumElements(), Op);
-
-    // Convert to correct type.
-    if (Op->getType() != IntIdxTy)
-      Op = Builder->CreateIntCast(Op, IntIdxTy, true, Op->getName().str()+".c");
-    if (Size != 1) {
-      // We'll let instcombine(mul) convert this to a shl if possible.
-      Op = Builder->CreateMul(Op, ConstantInt::get(IntIdxTy, Size),
-                              GEP->getName().str() + ".idx", false /*NUW*/,
-                              isInBounds /*NSW*/);
+    } else {
+      // Splat the index if needed.
+      if (IntIdxTy->isVectorTy() && !Op->getType()->isVectorTy())
+        Op = Builder->CreateVectorSplat(
+            cast<FixedVectorType>(IntIdxTy)->getNumElements(), Op);
+
+      // Convert to correct type.
+      if (Op->getType() != IntIdxTy)
+        Op = Builder->CreateIntCast(Op, IntIdxTy, true, Op->getName().str()+".c");
+      if (Size != 1) {
+        // We'll let instcombine(mul) convert this to a shl if possible.
+        Op = Builder->CreateMul(Op, ConstantInt::get(IntIdxTy, Size),
+                                GEP->getName().str() + ".idx", false /*NUW*/,
+                                isInBounds /*NSW*/);
+      }
+      Offset = Op;
     }
 
-    // Emit an add instruction.
-    Result = Builder->CreateAdd(Op, Result, GEP->getName().str()+".offs");
+    if (Result)
+      Result = Builder->CreateAdd(Result, Offset, GEP->getName().str()+".offs");
+    else
+      Result = Offset;
   }
-  return Result;
+  return Result ? Result : Constant::getNullValue(IntIdxTy);
 }
 
 }

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index abeddf45f22c..b8431a5a4532 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1672,13 +1672,11 @@ Value *InstCombinerImpl::OptimizePointerDifference(Value *LHS, Value *RHS,
   Value *Result = EmitGEPOffset(GEP1);
 
   // If this is a single inbounds GEP and the original sub was nuw,
-  // then the final multiplication is also nuw. We match an extra add zero
-  // here, because that's what EmitGEPOffset() generates.
-  Instruction *I;
-  if (IsNUW && !GEP2 && !Swapped && GEP1->isInBounds() &&
-      match(Result, m_Add(m_Instruction(I), m_Zero())) &&
-      I->getOpcode() == Instruction::Mul)
-    I->setHasNoUnsignedWrap();
+  // then the final multiplication is also nuw.
+  if (auto *I = dyn_cast<Instruction>(Result))
+    if (IsNUW && !GEP2 && !Swapped && GEP1->isInBounds() &&
+        I->getOpcode() == Instruction::Mul)
+      I->setHasNoUnsignedWrap();
 
   // If we had a constant expression GEP on the other side offsetting the
   // pointer, subtract it from the offset we have.

diff  --git a/llvm/test/Transforms/InstCombine/sub-gep.ll b/llvm/test/Transforms/InstCombine/sub-gep.ll
index ce9657433bb7..47dd0ce72f83 100644
--- a/llvm/test/Transforms/InstCombine/sub-gep.ll
+++ b/llvm/test/Transforms/InstCombine/sub-gep.ll
@@ -143,7 +143,7 @@ define i64 @test_inbounds_nuw_multi_index([0 x [2 x i32]]* %base, i64 %idx, i64
 ; CHECK-LABEL: @test_inbounds_nuw_multi_index(
 ; CHECK-NEXT:    [[P2_IDX:%.*]] = shl nsw i64 [[IDX:%.*]], 3
 ; CHECK-NEXT:    [[P2_IDX1:%.*]] = shl nsw i64 [[IDX2:%.*]], 2
-; CHECK-NEXT:    [[P2_OFFS2:%.*]] = add i64 [[P2_IDX1]], [[P2_IDX]]
+; CHECK-NEXT:    [[P2_OFFS2:%.*]] = add i64 [[P2_IDX]], [[P2_IDX1]]
 ; CHECK-NEXT:    ret i64 [[P2_OFFS2]]
 ;
   %p1 = getelementptr inbounds [0 x [2 x i32]], [0 x [2 x i32]]* %base, i64 0, i64 0, i64 0


        


More information about the llvm-commits mailing list