[SROA] Remove sroa-strict-inbounds option (NFC)

[nikic]

This option was added in 3b79b2a with the comment:

I had SROA implemented this way a long time ago and due to the
overwhelming bugs that surfaced, moved to a much more relaxed
variant. Richard Smith would like to understand the magnitude
of this problem and it seems fairly harmless to keep some
flag-controlled logic to get the extremely strict behavior here.
I'll remove it if it doesn't prove useful.

As far as I know, it did not prove useful, so I'm removing it now.

With constant GEPs canonicalized to i8, GEPs that only temporarily go out of bounds during the offset calculation do not naturally occur anymore anyway.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

This option was added in 3b79b2a with the comment:

> I had SROA implemented this way a long time ago and due to the
> overwhelming bugs that surfaced, moved to a much more relaxed
> variant. Richard Smith would like to understand the magnitude
> of this problem and it seems fairly harmless to keep some
> flag-controlled logic to get the extremely strict behavior here.
> I'll remove it if it doesn't prove useful.

As far as I know, it did not prove useful, so I'm removing it now.

With constant GEPs canonicalized to i8, GEPs that only temporarily go out of bounds during the offset calculation do not naturally occur anymore anyway.

---

Full diff: https://github.com/llvm/llvm-project/pull/94342.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/SROA.cpp (-43)

diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 756daf5bb41fa..a3df89b355640 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -116,10 +116,6 @@ STATISTIC(
 STATISTIC(NumDeleted, "Number of instructions deleted");
 STATISTIC(NumVectorized, "Number of vectorized aggregates");
 
-/// Hidden option to experiment with completely strict handling of inbounds
-/// GEPs.
-static cl::opt<bool> SROAStrictInbounds("sroa-strict-inbounds", cl::init(false),
-                                        cl::Hidden);
 /// Disable running mem2reg during SROA in order to test or debug SROA.
 static cl::opt<bool> SROASkipMem2Reg("sroa-skip-mem2reg", cl::init(false),
                                      cl::Hidden);
@@ -1095,45 +1091,6 @@ class AllocaSlices::SliceBuilder : public PtrUseVisitor<SliceBuilder> {
     if (GEPI.use_empty())
       return markAsDead(GEPI);
 
-    if (SROAStrictInbounds && GEPI.isInBounds()) {
-      // FIXME: This is a manually un-factored variant of the basic code inside
-      // of GEPs with checking of the inbounds invariant specified in the
-      // langref in a very strict sense. If we ever want to enable
-      // SROAStrictInbounds, this code should be factored cleanly into
-      // PtrUseVisitor, but it is easier to experiment with SROAStrictInbounds
-      // by writing out the code here where we have the underlying allocation
-      // size readily available.
-      APInt GEPOffset = Offset;
-      const DataLayout &DL = GEPI.getModule()->getDataLayout();
-      for (gep_type_iterator GTI = gep_type_begin(GEPI),
-                             GTE = gep_type_end(GEPI);
-           GTI != GTE; ++GTI) {
-        ConstantInt *OpC = dyn_cast<ConstantInt>(GTI.getOperand());
-        if (!OpC)
-          break;
-
-        // Handle a struct index, which adds its field offset to the pointer.
-        if (StructType *STy = GTI.getStructTypeOrNull()) {
-          unsigned ElementIdx = OpC->getZExtValue();
-          const StructLayout *SL = DL.getStructLayout(STy);
-          GEPOffset +=
-              APInt(Offset.getBitWidth(), SL->getElementOffset(ElementIdx));
-        } else {
-          // For array or vector indices, scale the index by the size of the
-          // type.
-          APInt Index = OpC->getValue().sextOrTrunc(Offset.getBitWidth());
-          GEPOffset += Index * APInt(Offset.getBitWidth(),
-                                     GTI.getSequentialElementStride(DL));
-        }
-
-        // If this index has computed an intermediate pointer which is not
-        // inbounds, then the result of the GEP is a poison value and we can
-        // delete it and all uses.
-        if (GEPOffset.ugt(AllocSize))
-          return markAsDead(GEPI);
-      }
-    }
-
     return Base::visitGetElementPtrInst(GEPI);
   }
 

[fhahn]

LGTM thanks! It indeed doesn't seem like this has been used for some time, also indicated by not having any tests at all.