Revert "[llvm] Use computeConstantRange to improve llvm.objectsize computation (llvm#114673)"

This reverts commit 5f34281.

This seems to break various builders, such as

https://lab.llvm.org/buildbot/#/builders/41/builds/3259
https://lab.llvm.org/buildbot/#/builders/76/builds/4298

Author: serge-sans-paille

llvm/include/llvm/Analysis/MemoryBuiltins.h

@@ -32,7 +32,6 @@ class AAResults;
 class Argument;
 class ConstantPointerNull;
 class DataLayout;
-class DominatorTree;
 class ExtractElementInst;
 class ExtractValueInst;
 class GEPOperator;
@@ -161,10 +160,8 @@ struct ObjectSizeOpts {
   /// though they can't be evaluated. Otherwise, null is always considered to
   /// point to a 0 byte region of memory.
   bool NullIsUnknownSize = false;
-  /// If set, used for more accurate evaluation.
+  /// If set, used for more accurate evaluation
   AAResults *AA = nullptr;
-  /// If set, used for more accurate evaluation.
-  DominatorTree *DT = nullptr;
 };
 
 /// Compute the size of the object pointed by Ptr. Returns true and the
@@ -189,12 +186,6 @@ Value *lowerObjectSizeCall(
     const TargetLibraryInfo *TLI, AAResults *AA, bool MustSucceed,
     SmallVectorImpl<Instruction *> *InsertedInstructions = nullptr);
 
-Value *lowerObjectSizeCall(
-    IntrinsicInst *ObjectSize, const DataLayout &DL,
-    const TargetLibraryInfo *TLI, AAResults *AA, DominatorTree *DT,
-    bool MustSucceed,
-    SmallVectorImpl<Instruction *> *InsertedInstructions = nullptr);
-
 /// SizeOffsetType - A base template class for the object size visitors. Used
 /// here as a self-documenting way to handle the values rather than using a
 /// \p std::pair.

llvm/include/llvm/IR/Value.h

@@ -723,16 +723,12 @@ class Value {
       bool AllowInvariantGroup = false,
       function_ref<bool(Value &Value, APInt &Offset)> ExternalAnalysis =
           nullptr) const;
-
-  Value *stripAndAccumulateConstantOffsets(
-      const DataLayout &DL, APInt &Offset, bool AllowNonInbounds,
-      bool AllowInvariantGroup = false,
-      function_ref<bool(Value &Value, APInt &Offset)> ExternalAnalysis =
-          nullptr) {
+  Value *stripAndAccumulateConstantOffsets(const DataLayout &DL, APInt &Offset,
+                                           bool AllowNonInbounds,
+                                           bool AllowInvariantGroup = false) {
     return const_cast<Value *>(
         static_cast<const Value *>(this)->stripAndAccumulateConstantOffsets(
-            DL, Offset, AllowNonInbounds, AllowInvariantGroup,
-            ExternalAnalysis));
+            DL, Offset, AllowNonInbounds, AllowInvariantGroup));
   }
 
   /// This is a wrapper around stripAndAccumulateConstantOffsets with the

llvm/lib/Analysis/MemoryBuiltins.cpp

@@ -25,7 +25,6 @@
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
-#include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalVariable.h"
@@ -590,28 +589,19 @@ Value *llvm::lowerObjectSizeCall(IntrinsicInst *ObjectSize,
                                  const TargetLibraryInfo *TLI,
                                  bool MustSucceed) {
   return lowerObjectSizeCall(ObjectSize, DL, TLI, /*AAResults=*/nullptr,
-                             /*DT=*/nullptr, MustSucceed);
+                             MustSucceed);
 }
 
 Value *llvm::lowerObjectSizeCall(
     IntrinsicInst *ObjectSize, const DataLayout &DL,
     const TargetLibraryInfo *TLI, AAResults *AA, bool MustSucceed,
     SmallVectorImpl<Instruction *> *InsertedInstructions) {
-  return lowerObjectSizeCall(ObjectSize, DL, TLI, AA, /*DT=*/nullptr,
-                             MustSucceed, InsertedInstructions);
-}
-
-Value *llvm::lowerObjectSizeCall(
-    IntrinsicInst *ObjectSize, const DataLayout &DL,
-    const TargetLibraryInfo *TLI, AAResults *AA, DominatorTree *DT,
-    bool MustSucceed, SmallVectorImpl<Instruction *> *InsertedInstructions) {
   assert(ObjectSize->getIntrinsicID() == Intrinsic::objectsize &&
          "ObjectSize must be a call to llvm.objectsize!");
 
   bool MaxVal = cast<ConstantInt>(ObjectSize->getArgOperand(1))->isZero();
   ObjectSizeOpts EvalOptions;
   EvalOptions.AA = AA;
-  EvalOptions.DT = DT;
 
   // Unless we have to fold this to something, try to be as accurate as
   // possible.
@@ -718,46 +708,14 @@ OffsetSpan ObjectSizeOffsetVisitor::computeImpl(Value *V) {
   // readjust the APInt as we pass it upwards in order for the APInt to match
   // the type the caller passed in.
   APInt Offset(InitialIntTyBits, 0);
-
-  // External Analysis used to compute the Min/Max value of individual Offsets
-  // within a GEP.
-  auto OffsetRangeAnalysis = [this, V](Value &VOffset, APInt &Offset) {
-    if (auto *C = dyn_cast<ConstantInt>(&VOffset)) {
-      Offset = C->getValue();
-      return true;
-    }
-    if (Options.EvalMode != ObjectSizeOpts::Mode::Min &&
-        Options.EvalMode != ObjectSizeOpts::Mode::Max) {
-      return false;
-    }
-    ConstantRange CR = computeConstantRange(
-        &VOffset, /*ForSigned*/ true, /*UseInstrInfo*/ true, /*AC=*/nullptr,
-        /*CtxtI=*/dyn_cast<Instruction>(V), /*DT=*/Options.DT);
-    if (CR.isFullSet())
-      return false;
-
-    if (Options.EvalMode == ObjectSizeOpts::Mode::Min) {
-      Offset = CR.getSignedMax();
-      // Upper bound actually unknown.
-      if (Offset.isMaxSignedValue())
-        return false;
-    } else {
-      Offset = CR.getSignedMin();
-      // Lower bound actually unknown.
-      if (Offset.isMinSignedValue())
-        return false;
-    }
-    return true;
-  };
-
   V = V->stripAndAccumulateConstantOffsets(
-      DL, Offset, /* AllowNonInbounds */ true, /* AllowInvariantGroup */ true,
-      /*ExternalAnalysis=*/OffsetRangeAnalysis);
+      DL, Offset, /* AllowNonInbounds */ true, /* AllowInvariantGroup */ true);
 
   // Later we use the index type size and zero but it will match the type of the
   // value that is passed to computeImpl.
   IntTyBits = DL.getIndexTypeSizeInBits(V->getType());
   Zero = APInt::getZero(IntTyBits);
+
   OffsetSpan ORT = computeValue(V);
 
   bool IndexTypeSizeChanged = InitialIntTyBits != IntTyBits;
@@ -835,26 +793,6 @@ OffsetSpan ObjectSizeOffsetVisitor::visitAllocaInst(AllocaInst &I) {
     Size = Size.umul_ov(NumElems, Overflow);
     return Overflow ? ObjectSizeOffsetVisitor::unknown()
                     : OffsetSpan(Zero, align(Size, I.getAlign()));
-  } else {
-    ConstantRange CR =
-        computeConstantRange(ArraySize, /*ForSigned*/ false,
-                             /*UseInstrInfo*/ true, /*AC=*/nullptr,
-                             /*CtxtI=*/&I, /*DT=*/Options.DT);
-    if (CR.isFullSet())
-      return ObjectSizeOffsetVisitor::unknown();
-    APInt Bound;
-    if (Options.EvalMode == ObjectSizeOpts::Mode::Max) {
-      Bound = CR.getUnsignedMax();
-      // Upper bound actually unknown.
-      if (Bound.isMaxValue())
-        return ObjectSizeOffsetVisitor::unknown();
-    } else {
-      Bound = CR.getUnsignedMin();
-      // Lower bound actually unknown.
-      if (Bound.isMinValue())
-        return ObjectSizeOffsetVisitor::unknown();
-    }
-    return OffsetSpan(Zero, align(Bound, I.getAlign()));
   }
   return ObjectSizeOffsetVisitor::unknown();
 }
@@ -872,32 +810,7 @@ OffsetSpan ObjectSizeOffsetVisitor::visitArgument(Argument &A) {
 }
 
 OffsetSpan ObjectSizeOffsetVisitor::visitCallBase(CallBase &CB) {
-  if (std::optional<APInt> Size =
-          getAllocSize(&CB, TLI, [&CB, this](const Value *V) -> const Value * {
-            if (!V->getType()->isIntegerTy())
-              return V;
-            if (isa<ConstantInt>(V))
-              return V;
-            ConstantRange CR = computeConstantRange(
-                V, /*ForSigned*/ false, /*UseInstrInfo*/ true, /*AC=*/nullptr,
-                /*CtxtI=*/&CB, /*DT=*/Options.DT);
-            if (CR.isFullSet())
-              return V;
-
-            APInt Bound;
-            if (Options.EvalMode == ObjectSizeOpts::Mode::Max) {
-              Bound = CR.getUnsignedMax();
-              // Upper bound actually unknown.
-              if (Bound.isMaxValue())
-                return V;
-            } else {
-              Bound = CR.getUnsignedMin();
-              // Lower bound actually unknown.
-              if (Bound.isMinValue())
-                return V;
-            }
-            return ConstantInt::get(V->getType(), Bound);
-          }))
+  if (std::optional<APInt> Size = getAllocSize(&CB, TLI))
     return OffsetSpan(Zero, *Size);
   return ObjectSizeOffsetVisitor::unknown();
 }

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

@@ -3318,9 +3318,8 @@ Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
       if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
         if (II->getIntrinsicID() == Intrinsic::objectsize) {
           SmallVector<Instruction *> InsertedInstructions;
-          Value *Result =
-              lowerObjectSizeCall(II, DL, &TLI, AA, &DT,
-                                  /*MustSucceed=*/true, &InsertedInstructions);
+          Value *Result = lowerObjectSizeCall(
+              II, DL, &TLI, AA, /*MustSucceed=*/true, &InsertedInstructions);
           for (Instruction *Inserted : InsertedInstructions)
             Worklist.add(Inserted);
           replaceInstUsesWith(*I, Result);

llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp

@@ -143,7 +143,7 @@ bool llvm::lowerConstantIntrinsics(Function &F, const TargetLibraryInfo &TLI,
       IsConstantIntrinsicsHandled++;
       break;
     case Intrinsic::objectsize:
-      NewValue = lowerObjectSizeCall(II, DL, &TLI, /*AA=*/nullptr, DT, true);
+      NewValue = lowerObjectSizeCall(II, DL, &TLI, true);
       LLVM_DEBUG(dbgs() << "Folding " << *II << " to " << *NewValue << "\n");
       ObjectSizeIntrinsicsHandled++;
       break;