[llvm] [InstCombine] Improve inbounds preservation for ADD+GEP -> GEP+GEP (PR #135155)

Björn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 14 01:32:02 PDT 2025


https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/135155

>From 6decf0ff33a065d7a46d7818e0272df3b086b008 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Fri, 11 Apr 2025 10:39:01 +0200
Subject: [PATCH 1/7] [InstCombine] Improve no-wrap flag preservation for
 ADD+GEP -> GEP+GEP

Given that we have a "add nuw" and a "getelementptr inbounds nuw" like
this:
   %idx = add nuw i64 %idx1, %idx2
   %gep = getelementptr inbounds nuw i32, ptr %ptr, i64 %idx

Then we can preserve the "inbounds nuw" flag when transforming that
into two getelementptr instructions:
   %gep1 = getelementptr inbounds nuw i32, ptr %ptr, i64 %idx1
   %gep = getelementptr inbounds nuw i32, ptr %ptr, i64 %idx2

Similarly for just having "nuw" or "nusw nuw" instead of "inbounds nuw"
on the getelementptr.

Proof: https://alive2.llvm.org/ce/z/QSweWW
---
 .../InstCombine/InstructionCombining.cpp      | 43 +++++++++++--------
 llvm/test/Transforms/InstCombine/array.ll     | 14 +++---
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 856e02c9f1ddb..19a818f4baa30 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3087,12 +3087,22 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
     return nullptr;
 
   if (GEP.getNumIndices() == 1) {
-    // We can only preserve inbounds if the original gep is inbounds, the add
-    // is nsw, and the add operands are non-negative.
-    auto CanPreserveInBounds = [&](bool AddIsNSW, Value *Idx1, Value *Idx2) {
+    auto CanPreserveNoWrapFlags = [&](bool AddIsNSW, bool AddIsNUW, Value *Idx1,
+                                      Value *Idx2) {
+      // Preserve "inbounds nuw" if the original gep is "inbounds nuw",
+      // and the add is "nuw".
+      if (GEP.isInBounds() && GEP.hasNoUnsignedWrap() && AddIsNUW)
+        return GEPNoWrapFlags::inBounds() | GEPNoWrapFlags::noUnsignedWrap();
+      // Preserve "inbounds" if the original gep is "inbounds", the add
+      // is "nsw", and the add operands are non-negative.
       SimplifyQuery Q = SQ.getWithInstruction(&GEP);
-      return GEP.isInBounds() && AddIsNSW && isKnownNonNegative(Idx1, Q) &&
-             isKnownNonNegative(Idx2, Q);
+      if (GEP.isInBounds() && AddIsNSW && isKnownNonNegative(Idx1, Q) &&
+          isKnownNonNegative(Idx2, Q))
+        return GEPNoWrapFlags::inBounds();
+      // Preserve "nuw" if the original gep is "nuw", and the add is "nuw".
+      if (GEP.hasNoUnsignedWrap() && AddIsNUW)
+        return GEPNoWrapFlags::noUnsignedWrap();
+      return GEPNoWrapFlags::none();
     };
 
     // Try to replace ADD + GEP with GEP + GEP.
@@ -3104,15 +3114,15 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       // as:
       //   %newptr = getelementptr i32, ptr %ptr, i64 %idx1
       //   %newgep = getelementptr i32, ptr %newptr, i64 %idx2
-      bool IsInBounds = CanPreserveInBounds(
-          cast<OverflowingBinaryOperator>(GEP.getOperand(1))->hasNoSignedWrap(),
-          Idx1, Idx2);
+      bool NSW = match(GEP.getOperand(1), m_NSWAddLike(m_Value(), m_Value()));
+      bool NUW = match(GEP.getOperand(1), m_NUWAddLike(m_Value(), m_Value()));
+      GEPNoWrapFlags NWFlags = CanPreserveNoWrapFlags(NSW, NUW, Idx1, Idx2);
       auto *NewPtr =
           Builder.CreateGEP(GEP.getSourceElementType(), GEP.getPointerOperand(),
-                            Idx1, "", IsInBounds);
-      return replaceInstUsesWith(
-          GEP, Builder.CreateGEP(GEP.getSourceElementType(), NewPtr, Idx2, "",
-                                 IsInBounds));
+                            Idx1, "", NWFlags);
+      return replaceInstUsesWith(GEP,
+                                 Builder.CreateGEP(GEP.getSourceElementType(),
+                                                   NewPtr, Idx2, "", NWFlags));
     }
     ConstantInt *C;
     if (match(GEP.getOperand(1), m_OneUse(m_SExtLike(m_OneUse(m_NSWAdd(
@@ -3123,17 +3133,16 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       // as:
       // %newptr = getelementptr i32, ptr %ptr, i32 %idx1
       // %newgep = getelementptr i32, ptr %newptr, i32 idx2
-      bool IsInBounds = CanPreserveInBounds(
-          /*IsNSW=*/true, Idx1, C);
+      GEPNoWrapFlags NWFlags = CanPreserveNoWrapFlags(
+          /*IsNSW=*/true, /*IsNUW=*/false, Idx1, C);
       auto *NewPtr = Builder.CreateGEP(
           GEP.getSourceElementType(), GEP.getPointerOperand(),
-          Builder.CreateSExt(Idx1, GEP.getOperand(1)->getType()), "",
-          IsInBounds);
+          Builder.CreateSExt(Idx1, GEP.getOperand(1)->getType()), "", NWFlags);
       return replaceInstUsesWith(
           GEP,
           Builder.CreateGEP(GEP.getSourceElementType(), NewPtr,
                             Builder.CreateSExt(C, GEP.getOperand(1)->getType()),
-                            "", IsInBounds));
+                            "", NWFlags));
     }
   }
 
diff --git a/llvm/test/Transforms/InstCombine/array.ll b/llvm/test/Transforms/InstCombine/array.ll
index a09b66273368c..aa32c094c9e27 100644
--- a/llvm/test/Transforms/InstCombine/array.ll
+++ b/llvm/test/Transforms/InstCombine/array.ll
@@ -122,12 +122,11 @@ define ptr @gep_inbounds_nuwaddlike(ptr %ptr, i64 %a, i64 %b) {
   ret ptr %gep
 }
 
-; FIXME: Preserve "inbounds nuw".
 define ptr @gep_inbounds_add_nuw(ptr %ptr, i64 %a, i64 %b) {
 ; CHECK-LABEL: define ptr @gep_inbounds_add_nuw(
 ; CHECK-SAME: ptr [[PTR:%.*]], i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr i32, ptr [[PTR]], i64 [[A]]
-; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i32, ptr [[TMP1]], i64 [[B]]
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr inbounds nuw i32, ptr [[PTR]], i64 [[A]]
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds nuw i32, ptr [[TMP1]], i64 [[B]]
 ; CHECK-NEXT:    ret ptr [[GEP]]
 ;
   %add = add nuw i64 %a, %b
@@ -139,8 +138,8 @@ define ptr @gep_inbounds_add_nuw(ptr %ptr, i64 %a, i64 %b) {
 define ptr @gep_inbounds_add_nusw_nuw(ptr %ptr, i64 %a, i64 %b) {
 ; CHECK-LABEL: define ptr @gep_inbounds_add_nusw_nuw(
 ; CHECK-SAME: ptr [[PTR:%.*]], i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr i32, ptr [[PTR]], i64 [[A]]
-; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i32, ptr [[TMP1]], i64 [[B]]
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr nuw i32, ptr [[PTR]], i64 [[A]]
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr nuw i32, ptr [[TMP1]], i64 [[B]]
 ; CHECK-NEXT:    ret ptr [[GEP]]
 ;
   %add = add nuw i64 %a, %b
@@ -148,12 +147,11 @@ define ptr @gep_inbounds_add_nusw_nuw(ptr %ptr, i64 %a, i64 %b) {
   ret ptr %gep
 }
 
-; FIXME: Preserve "nuw".
 define ptr @gep_add_nuw(ptr %ptr, i64 %a, i64 %b) {
 ; CHECK-LABEL: define ptr @gep_add_nuw(
 ; CHECK-SAME: ptr [[PTR:%.*]], i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr i32, ptr [[PTR]], i64 [[A]]
-; CHECK-NEXT:    [[GEP:%.*]] = getelementptr i32, ptr [[TMP1]], i64 [[B]]
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr nuw i32, ptr [[PTR]], i64 [[A]]
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr nuw i32, ptr [[TMP1]], i64 [[B]]
 ; CHECK-NEXT:    ret ptr [[GEP]]
 ;
   %add = add nuw i64 %a, %b

>From b0ee9750327de75ae07119940292922e2d7be5d4 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Thu, 10 Apr 2025 15:00:34 +0200
Subject: [PATCH 2/7] Fixups after review feedback:

- Make sure we try to derive inbound + nuw before doing the
  ADD+GEP->GEP+GEP rewrites. This to make it possible to rely
  on "nuw" being present when trying to preserve flags.

- Make sure the special ADDNSW+SEXTLIKE+GEP->GEP+GEP case is checking
  for ADDNUW+ZEXTNNEG+GEP->GEP+GEP when preserving flags.
  This way we do not need to check for non-negative operands.
---
 .../InstCombine/InstructionCombining.cpp      | 99 +++++++++----------
 1 file changed, 48 insertions(+), 51 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 19a818f4baa30..b5e085be9b084 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3086,23 +3086,51 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
   if (GEPType->isVectorTy())
     return nullptr;
 
+  if (!GEP.isInBounds()) {
+    unsigned IdxWidth =
+        DL.getIndexSizeInBits(PtrOp->getType()->getPointerAddressSpace());
+    APInt BasePtrOffset(IdxWidth, 0);
+    Value *UnderlyingPtrOp =
+            PtrOp->stripAndAccumulateInBoundsConstantOffsets(DL,
+                                                             BasePtrOffset);
+    bool CanBeNull, CanBeFreed;
+    uint64_t DerefBytes = UnderlyingPtrOp->getPointerDereferenceableBytes(
+        DL, CanBeNull, CanBeFreed);
+    if (!CanBeNull && !CanBeFreed && DerefBytes != 0) {
+      if (GEP.accumulateConstantOffset(DL, BasePtrOffset) &&
+          BasePtrOffset.isNonNegative()) {
+        APInt AllocSize(IdxWidth, DerefBytes);
+        if (BasePtrOffset.ule(AllocSize)) {
+          return GetElementPtrInst::CreateInBounds(
+              GEP.getSourceElementType(), PtrOp, Indices, GEP.getName());
+        }
+      }
+    }
+  }
+
+  // nusw + nneg -> nuw
+  if (GEP.hasNoUnsignedSignedWrap() && !GEP.hasNoUnsignedWrap() &&
+      all_of(GEP.indices(), [&](Value *Idx) {
+        return isKnownNonNegative(Idx, SQ.getWithInstruction(&GEP));
+      })) {
+    GEP.setNoWrapFlags(GEP.getNoWrapFlags() | GEPNoWrapFlags::noUnsignedWrap());
+    return &GEP;
+  }
+
+  // These rewrites is trying to preserve inbounds/nuw attributes. So we want to
+  // do this after having tried to derive "nuw" above.
   if (GEP.getNumIndices() == 1) {
-    auto CanPreserveNoWrapFlags = [&](bool AddIsNSW, bool AddIsNUW, Value *Idx1,
-                                      Value *Idx2) {
-      // Preserve "inbounds nuw" if the original gep is "inbounds nuw",
-      // and the add is "nuw".
-      if (GEP.isInBounds() && GEP.hasNoUnsignedWrap() && AddIsNUW)
-        return GEPNoWrapFlags::inBounds() | GEPNoWrapFlags::noUnsignedWrap();
-      // Preserve "inbounds" if the original gep is "inbounds", the add
-      // is "nsw", and the add operands are non-negative.
-      SimplifyQuery Q = SQ.getWithInstruction(&GEP);
-      if (GEP.isInBounds() && AddIsNSW && isKnownNonNegative(Idx1, Q) &&
-          isKnownNonNegative(Idx2, Q))
-        return GEPNoWrapFlags::inBounds();
-      // Preserve "nuw" if the original gep is "nuw", and the add is "nuw".
-      if (GEP.hasNoUnsignedWrap() && AddIsNUW)
-        return GEPNoWrapFlags::noUnsignedWrap();
-      return GEPNoWrapFlags::none();
+    auto GetPreservedNoWrapFlags = [&](bool AddIsNUW, Value *Idx1, Value *Idx2) {
+      // Preserve "inbounds nuw" if the original gep is "inbounds nuw", and the
+      // add is "nuw". Preserve "nuw" if the original gep is "nuw", and the add
+      // is "nuw".
+      GEPNoWrapFlags Flags = GEPNoWrapFlags::none();
+      if (GEP.hasNoUnsignedWrap() && AddIsNUW) {
+        Flags |= GEPNoWrapFlags::noUnsignedWrap();
+        if (GEP.isInBounds())
+          Flags |= GEPNoWrapFlags::inBounds();
+      }
+      return Flags;
     };
 
     // Try to replace ADD + GEP with GEP + GEP.
@@ -3114,9 +3142,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       // as:
       //   %newptr = getelementptr i32, ptr %ptr, i64 %idx1
       //   %newgep = getelementptr i32, ptr %newptr, i64 %idx2
-      bool NSW = match(GEP.getOperand(1), m_NSWAddLike(m_Value(), m_Value()));
       bool NUW = match(GEP.getOperand(1), m_NUWAddLike(m_Value(), m_Value()));
-      GEPNoWrapFlags NWFlags = CanPreserveNoWrapFlags(NSW, NUW, Idx1, Idx2);
+      GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW, Idx1, Idx2);
       auto *NewPtr =
           Builder.CreateGEP(GEP.getSourceElementType(), GEP.getPointerOperand(),
                             Idx1, "", NWFlags);
@@ -3133,8 +3160,9 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       // as:
       // %newptr = getelementptr i32, ptr %ptr, i32 %idx1
       // %newgep = getelementptr i32, ptr %newptr, i32 idx2
-      GEPNoWrapFlags NWFlags = CanPreserveNoWrapFlags(
-          /*IsNSW=*/true, /*IsNUW=*/false, Idx1, C);
+      bool NUW = match(GEP.getOperand(1), m_NNegZExt(m_NUWAddLike(m_Value(),
+                                                                  m_Value())));
+      GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW, Idx1, C);
       auto *NewPtr = Builder.CreateGEP(
           GEP.getSourceElementType(), GEP.getPointerOperand(),
           Builder.CreateSExt(Idx1, GEP.getOperand(1)->getType()), "", NWFlags);
@@ -3146,37 +3174,6 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
     }
   }
 
-  if (!GEP.isInBounds()) {
-    unsigned IdxWidth =
-        DL.getIndexSizeInBits(PtrOp->getType()->getPointerAddressSpace());
-    APInt BasePtrOffset(IdxWidth, 0);
-    Value *UnderlyingPtrOp =
-            PtrOp->stripAndAccumulateInBoundsConstantOffsets(DL,
-                                                             BasePtrOffset);
-    bool CanBeNull, CanBeFreed;
-    uint64_t DerefBytes = UnderlyingPtrOp->getPointerDereferenceableBytes(
-        DL, CanBeNull, CanBeFreed);
-    if (!CanBeNull && !CanBeFreed && DerefBytes != 0) {
-      if (GEP.accumulateConstantOffset(DL, BasePtrOffset) &&
-          BasePtrOffset.isNonNegative()) {
-        APInt AllocSize(IdxWidth, DerefBytes);
-        if (BasePtrOffset.ule(AllocSize)) {
-          return GetElementPtrInst::CreateInBounds(
-              GEP.getSourceElementType(), PtrOp, Indices, GEP.getName());
-        }
-      }
-    }
-  }
-
-  // nusw + nneg -> nuw
-  if (GEP.hasNoUnsignedSignedWrap() && !GEP.hasNoUnsignedWrap() &&
-      all_of(GEP.indices(), [&](Value *Idx) {
-        return isKnownNonNegative(Idx, SQ.getWithInstruction(&GEP));
-      })) {
-    GEP.setNoWrapFlags(GEP.getNoWrapFlags() | GEPNoWrapFlags::noUnsignedWrap());
-    return &GEP;
-  }
-
   if (Instruction *R = foldSelectGEP(GEP, Builder))
     return R;
 

>From 7d7f3b92408354cb786e28074ca7842f5083bbfc Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Thu, 10 Apr 2025 16:34:51 +0200
Subject: [PATCH 3/7] Fix clang-format

---
 .../Transforms/InstCombine/InstructionCombining.cpp    | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index b5e085be9b084..09b6f4880fa16 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3091,8 +3091,7 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
         DL.getIndexSizeInBits(PtrOp->getType()->getPointerAddressSpace());
     APInt BasePtrOffset(IdxWidth, 0);
     Value *UnderlyingPtrOp =
-            PtrOp->stripAndAccumulateInBoundsConstantOffsets(DL,
-                                                             BasePtrOffset);
+        PtrOp->stripAndAccumulateInBoundsConstantOffsets(DL, BasePtrOffset);
     bool CanBeNull, CanBeFreed;
     uint64_t DerefBytes = UnderlyingPtrOp->getPointerDereferenceableBytes(
         DL, CanBeNull, CanBeFreed);
@@ -3120,7 +3119,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
   // These rewrites is trying to preserve inbounds/nuw attributes. So we want to
   // do this after having tried to derive "nuw" above.
   if (GEP.getNumIndices() == 1) {
-    auto GetPreservedNoWrapFlags = [&](bool AddIsNUW, Value *Idx1, Value *Idx2) {
+    auto GetPreservedNoWrapFlags = [&](bool AddIsNUW, Value *Idx1,
+                                       Value *Idx2) {
       // Preserve "inbounds nuw" if the original gep is "inbounds nuw", and the
       // add is "nuw". Preserve "nuw" if the original gep is "nuw", and the add
       // is "nuw".
@@ -3160,8 +3160,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       // as:
       // %newptr = getelementptr i32, ptr %ptr, i32 %idx1
       // %newgep = getelementptr i32, ptr %newptr, i32 idx2
-      bool NUW = match(GEP.getOperand(1), m_NNegZExt(m_NUWAddLike(m_Value(),
-                                                                  m_Value())));
+      bool NUW = match(GEP.getOperand(1),
+                       m_NNegZExt(m_NUWAddLike(m_Value(), m_Value())));
       GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW, Idx1, C);
       auto *NewPtr = Builder.CreateGEP(
           GEP.getSourceElementType(), GEP.getPointerOperand(),

>From 205a631fb3fe2acfab0e35e7dc3c48205b0a0e54 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Fri, 11 Apr 2025 10:07:20 +0200
Subject: [PATCH 4/7] Fixup

---
 llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 09b6f4880fa16..14fecc1f090c0 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3119,8 +3119,7 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
   // These rewrites is trying to preserve inbounds/nuw attributes. So we want to
   // do this after having tried to derive "nuw" above.
   if (GEP.getNumIndices() == 1) {
-    auto GetPreservedNoWrapFlags = [&](bool AddIsNUW, Value *Idx1,
-                                       Value *Idx2) {
+    auto GetPreservedNoWrapFlags = [&](bool AddIsNUW) {
       // Preserve "inbounds nuw" if the original gep is "inbounds nuw", and the
       // add is "nuw". Preserve "nuw" if the original gep is "nuw", and the add
       // is "nuw".
@@ -3143,7 +3142,7 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       //   %newptr = getelementptr i32, ptr %ptr, i64 %idx1
       //   %newgep = getelementptr i32, ptr %newptr, i64 %idx2
       bool NUW = match(GEP.getOperand(1), m_NUWAddLike(m_Value(), m_Value()));
-      GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW, Idx1, Idx2);
+      GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW);
       auto *NewPtr =
           Builder.CreateGEP(GEP.getSourceElementType(), GEP.getPointerOperand(),
                             Idx1, "", NWFlags);
@@ -3162,7 +3161,7 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       // %newgep = getelementptr i32, ptr %newptr, i32 idx2
       bool NUW = match(GEP.getOperand(1),
                        m_NNegZExt(m_NUWAddLike(m_Value(), m_Value())));
-      GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW, Idx1, C);
+      GEPNoWrapFlags NWFlags = GetPreservedNoWrapFlags(NUW);
       auto *NewPtr = Builder.CreateGEP(
           GEP.getSourceElementType(), GEP.getPointerOperand(),
           Builder.CreateSExt(Idx1, GEP.getOperand(1)->getType()), "", NWFlags);

>From 5a14ee7f8a55a8f482bbbcfbe74f0dc2d56a1ff1 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Fri, 11 Apr 2025 12:52:44 +0200
Subject: [PATCH 5/7] Fixup: Also handle "nusw nuw"

---
 .../InstCombine/InstructionCombining.cpp        | 17 +++++++----------
 llvm/test/Transforms/InstCombine/array.ll       |  5 ++---
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 14fecc1f090c0..39ea5e111ec18 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3119,17 +3119,14 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
   // These rewrites is trying to preserve inbounds/nuw attributes. So we want to
   // do this after having tried to derive "nuw" above.
   if (GEP.getNumIndices() == 1) {
+    // Given (gep p, x+y) we want to determine the common nowrap flags for both
+    // gep if transforming into (gep (gep p, x), y).
     auto GetPreservedNoWrapFlags = [&](bool AddIsNUW) {
-      // Preserve "inbounds nuw" if the original gep is "inbounds nuw", and the
-      // add is "nuw". Preserve "nuw" if the original gep is "nuw", and the add
-      // is "nuw".
-      GEPNoWrapFlags Flags = GEPNoWrapFlags::none();
-      if (GEP.hasNoUnsignedWrap() && AddIsNUW) {
-        Flags |= GEPNoWrapFlags::noUnsignedWrap();
-        if (GEP.isInBounds())
-          Flags |= GEPNoWrapFlags::inBounds();
-      }
-      return Flags;
+      // We can preserve both "inbounds nuw", "nusw nuw" and "nuw" if we know
+      // that x + y does not have unsigned wrap.
+      if (GEP.hasNoUnsignedWrap() && AddIsNUW)
+        return GEP.getNoWrapFlags();
+      return GEPNoWrapFlags::none();
     };
 
     // Try to replace ADD + GEP with GEP + GEP.
diff --git a/llvm/test/Transforms/InstCombine/array.ll b/llvm/test/Transforms/InstCombine/array.ll
index aa32c094c9e27..dd4725cfb1db3 100644
--- a/llvm/test/Transforms/InstCombine/array.ll
+++ b/llvm/test/Transforms/InstCombine/array.ll
@@ -134,12 +134,11 @@ define ptr @gep_inbounds_add_nuw(ptr %ptr, i64 %a, i64 %b) {
   ret ptr %gep
 }
 
-; FIXME: Preserve "nusw nuw".
 define ptr @gep_inbounds_add_nusw_nuw(ptr %ptr, i64 %a, i64 %b) {
 ; CHECK-LABEL: define ptr @gep_inbounds_add_nusw_nuw(
 ; CHECK-SAME: ptr [[PTR:%.*]], i64 [[A:%.*]], i64 [[B:%.*]]) {
-; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr nuw i32, ptr [[PTR]], i64 [[A]]
-; CHECK-NEXT:    [[GEP:%.*]] = getelementptr nuw i32, ptr [[TMP1]], i64 [[B]]
+; CHECK-NEXT:    [[TMP1:%.*]] = getelementptr nusw nuw i32, ptr [[PTR]], i64 [[A]]
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr nusw nuw i32, ptr [[TMP1]], i64 [[B]]
 ; CHECK-NEXT:    ret ptr [[GEP]]
 ;
   %add = add nuw i64 %a, %b

>From a58d4daf86dbf26890cf073b0979512d6768f125 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Fri, 11 Apr 2025 15:41:31 +0200
Subject: [PATCH 6/7] Fixup code comments

---
 llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 39ea5e111ec18..d0d7e2d3e52f0 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3116,11 +3116,11 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
     return &GEP;
   }
 
-  // These rewrites is trying to preserve inbounds/nuw attributes. So we want to
+  // These rewrites are trying to preserve inbounds/nuw attributes. So we want to
   // do this after having tried to derive "nuw" above.
   if (GEP.getNumIndices() == 1) {
     // Given (gep p, x+y) we want to determine the common nowrap flags for both
-    // gep if transforming into (gep (gep p, x), y).
+    // geps if transforming into (gep (gep p, x), y).
     auto GetPreservedNoWrapFlags = [&](bool AddIsNUW) {
       // We can preserve both "inbounds nuw", "nusw nuw" and "nuw" if we know
       // that x + y does not have unsigned wrap.

>From dd8cdafdc7bf0ffc3b7b1a2778634fd95095f373 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Fri, 11 Apr 2025 15:47:21 +0200
Subject: [PATCH 7/7] Fixup formatting

---
 llvm/lib/Transforms/InstCombine/InstructionCombining.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index d0d7e2d3e52f0..48f9179d5eaab 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3116,8 +3116,8 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
     return &GEP;
   }
 
-  // These rewrites are trying to preserve inbounds/nuw attributes. So we want to
-  // do this after having tried to derive "nuw" above.
+  // These rewrites are trying to preserve inbounds/nuw attributes. So we want
+  // to do this after having tried to derive "nuw" above.
   if (GEP.getNumIndices() == 1) {
     // Given (gep p, x+y) we want to determine the common nowrap flags for both
     // geps if transforming into (gep (gep p, x), y).



More information about the llvm-commits mailing list