[llvm] [SROA] Remove sroa-strict-inbounds option (NFC) (PR #94342)

[Nikita Popov via llvm-commits]

https://github.com/nikic created https://github.com/llvm/llvm-project/pull/94342

This option was added in 3b79b2ab4e35353e63ba323a3de4b0a70c61a5f1 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.

>From 088d6089ecf3d4e19eee209f3623163d36344b6b Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 4 Jun 2024 14:10:19 +0200
Subject: [PATCH] [SROA] Remove sroa-strict-inbounds option (NFC)

This option was added in 3b79b2ab4e35353e63ba323a3de4b0a70c61a5f1
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.
---
 llvm/lib/Transforms/Scalar/SROA.cpp | 43 -----------------------------
 1 file changed, 43 deletions(-)

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);
   }