Skip to content

Conversation

@vidsinghal
Copy link
Contributor

@vidsinghal vidsinghal commented Nov 11, 2023

This PR changes the allocation size of objects using AAPointerInfo. In case the allocation size changes, it will change the load/store offsets as needed by inserting a new gep that loads from a shifted offset.

@llvmbot llvmbot added llvm:transforms clang:openmp OpenMP related changes to Clang labels Nov 11, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 11, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Vidhush Singhal (vidsinghal)

Changes

Work in progress

a.) Need to fix some TODOs (if offsets need to be changed for Load and Store instructions)
b.) For some tests assertions fail

opt: llvm-project/llvm/include/llvm/Transforms/IPO/Attributor.h:1870: bool llvm::Attributor::changeAfterManifest(const IRPosition, Value &, bool): Assertion `(!CurNV || CurNV == &NV || isa<UndefValue>(NV)) && "Value replacement was registered twice with different values!"' failed.

Patch is 295.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72029.diff

46 Files Affected:

  • (modified) llvm/include/llvm/Transforms/IPO/Attributor.h (+48)
  • (modified) llvm/lib/Transforms/IPO/Attributor.cpp (+12-3)
  • (modified) llvm/lib/Transforms/IPO/AttributorAttributes.cpp (+344-8)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/2008-02-01-ReturnAttrs.ll (+8-8)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/X86/attributes.ll (+24-24)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/X86/min-legal-vector-width.ll (+104-104)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/alignment.ll (+3-3)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/alloca-as.ll (+5-5)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/array.ll (+6-6)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/attrs.ll (+12-12)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/basictest.ll (+1-1)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/byval-2.ll (+11-11)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/byval.ll (+19-19)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/control-flow2.ll (+1-1)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll (+5-1)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll (+1-1)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll (+5-3)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead_2.ll (+6-4)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll (+1)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/pr33641_remove_arg_dbgvalue.ll (+1-1)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/profile.ll (+3-3)
  • (modified) llvm/test/Transforms/Attributor/ArgumentPromotion/sret.ll (+3-2)
  • (modified) llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll (+35-35)
  • (modified) llvm/test/Transforms/Attributor/IPConstantProp/pthreads.ll (+4-4)
  • (modified) llvm/test/Transforms/Attributor/IPConstantProp/return-argument.ll (+2-2)
  • (modified) llvm/test/Transforms/Attributor/align.ll (+33-20)
  • (added) llvm/test/Transforms/Attributor/allocator.ll (+524)
  • (modified) llvm/test/Transforms/Attributor/callbacks.ll (+29-29)
  • (modified) llvm/test/Transforms/Attributor/heap_to_stack.ll (+5-4)
  • (modified) llvm/test/Transforms/Attributor/heap_to_stack_gpu.ll (+5-4)
  • (modified) llvm/test/Transforms/Attributor/internal-noalias.ll (+7-7)
  • (modified) llvm/test/Transforms/Attributor/liveness.ll (+8-8)
  • (modified) llvm/test/Transforms/Attributor/noalias.ll (+13-13)
  • (modified) llvm/test/Transforms/Attributor/nocapture-2.ll (+22-22)
  • (modified) llvm/test/Transforms/Attributor/nodelete.ll (+2)
  • (modified) llvm/test/Transforms/Attributor/nofpclass.ll (+66-18)
  • (modified) llvm/test/Transforms/Attributor/norecurse.ll (+3-3)
  • (modified) llvm/test/Transforms/Attributor/openmp_parallel.ll (+6-6)
  • (modified) llvm/test/Transforms/Attributor/pointer-info.ll (+3-3)
  • (modified) llvm/test/Transforms/Attributor/value-simplify-assume.ll (+110-110)
  • (modified) llvm/test/Transforms/Attributor/value-simplify-dominance.ll (+12-12)
  • (modified) llvm/test/Transforms/Attributor/value-simplify-gpu.ll (+3-3)
  • (modified) llvm/test/Transforms/Attributor/value-simplify-local-remote.ll (+8-8)
  • (modified) llvm/test/Transforms/Attributor/value-simplify.ll (+15-15)
  • (modified) llvm/test/Transforms/OpenMP/parallel_deletion.ll (+55-12)
  • (modified) llvm/test/Transforms/OpenMP/parallel_region_merging.ll (+225-197)
diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index bd1bd8261123e51..8488568113d553c 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -103,6 +103,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/iterator.h"
 #include "llvm/Analysis/AssumeBundleQueries.h"
 #include "llvm/Analysis/CFG.h"
@@ -132,6 +133,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ModRef.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/TypeSize.h"
 #include "llvm/TargetParser/Triple.h"
 #include "llvm/Transforms/Utils/CallGraphUpdater.h"
 
@@ -6117,6 +6119,14 @@ struct AAPointerInfo : public AbstractAttribute {
   /// See AbstractAttribute::getIdAddr()
   const char *getIdAddr() const override { return &ID; }
 
+  using OffsetBinsTy = DenseMap<AA::RangeTy, SmallSet<unsigned, 4>>;
+  using const_bin_iterator = OffsetBinsTy::const_iterator;
+  virtual const_bin_iterator begin() const = 0;
+  virtual const_bin_iterator end() const = 0;
+  virtual int64_t numOffsetBins() const = 0;
+  virtual void dumpState(raw_ostream &O) const = 0;
+  virtual const Access &getBinAccess(unsigned Index) const = 0;
+
   /// Call \p CB on all accesses that might interfere with \p Range and return
   /// true if all such accesses were known and the callback returned true for
   /// all of them, false otherwise. An access interferes with an offset-size
@@ -6270,6 +6280,44 @@ struct AAAddressSpace : public StateWrapper<BooleanState, AbstractAttribute> {
   static const char ID;
 };
 
+struct AAAllocationInfo : public StateWrapper<BooleanState, AbstractAttribute> {
+  AAAllocationInfo(const IRPosition &IRP, Attributor &A)
+      : StateWrapper<BooleanState, AbstractAttribute>(IRP) {}
+
+  /// See AbstractAttribute::isValidIRPositionForInit
+  static bool isValidIRPositionForInit(Attributor &A, const IRPosition &IRP) {
+    if (!IRP.getAssociatedType()->isPtrOrPtrVectorTy())
+      return false;
+    return AbstractAttribute::isValidIRPositionForInit(A, IRP);
+  }
+
+  /// Create an abstract attribute view for the position \p IRP.
+  static AAAllocationInfo &createForPosition(const IRPosition &IRP,
+                                             Attributor &A);
+
+  virtual std::optional<TypeSize> getAllocatedSize() const = 0;
+
+  using NewOffsetsTy = DenseMap<AA::RangeTy, AA::RangeTy>;
+  virtual const NewOffsetsTy &getNewOffsets() const = 0;
+
+  /// See AbstractAttribute::getName()
+  const std::string getName() const override { return "AAAllocationInfo"; }
+
+  /// See AbstractAttribute::getIdAddr()
+  const char *getIdAddr() const override { return &ID; }
+
+  /// This function should return true if the type of the \p AA is
+  /// AAAllocationInfo
+  static bool classof(const AbstractAttribute *AA) {
+    return (AA->getIdAddr() == &ID);
+  }
+
+  constexpr static const std::optional<TypeSize> HasNoAllocationSize =
+      std::optional<TypeSize>(TypeSize(-1, true));
+
+  static const char ID;
+};
+
 /// An abstract interface for llvm::GlobalValue information interference.
 struct AAGlobalValueInfo
     : public StateWrapper<BooleanState, AbstractAttribute> {
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 49ced893d5c7340..f1a88bc564ced71 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -3611,14 +3611,13 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
   };
 
   auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
-  bool Success;
+  [[maybe_unused]] bool Success;
   bool UsedAssumedInformation = false;
   Success = checkForAllInstructionsImpl(
       nullptr, OpcodeInstMap, CallSitePred, nullptr, nullptr,
       {(unsigned)Instruction::Invoke, (unsigned)Instruction::CallBr,
        (unsigned)Instruction::Call},
       UsedAssumedInformation);
-  (void)Success;
   assert(Success && "Expected the check call to be successful!");
 
   auto LoadStorePred = [&](Instruction &I) -> bool {
@@ -3644,7 +3643,17 @@ void Attributor::identifyDefaultAbstractAttributes(Function &F) {
       nullptr, OpcodeInstMap, LoadStorePred, nullptr, nullptr,
       {(unsigned)Instruction::Load, (unsigned)Instruction::Store},
       UsedAssumedInformation);
-  (void)Success;
+  assert(Success && "Expected the check call to be successful!");
+
+  // AllocaInstPredicate
+  auto AAAllocationInfoPred = [&](Instruction &I) -> bool {
+    getOrCreateAAFor<AAAllocationInfo>(IRPosition::value(I));
+    return true;
+  };
+
+  Success = checkForAllInstructionsImpl(
+      nullptr, OpcodeInstMap, AAAllocationInfoPred, nullptr, nullptr,
+      {(unsigned)Instruction::Alloca}, UsedAssumedInformation);
   assert(Success && "Expected the check call to be successful!");
 }
 
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index bbb0cfa0eb05fe6..dea00c0039798b2 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -65,6 +65,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/GraphWriter.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/TypeSize.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/CallPromotionUtils.h"
@@ -192,6 +193,7 @@ PIPE_OPERATOR(AAPointerInfo)
 PIPE_OPERATOR(AAAssumptionInfo)
 PIPE_OPERATOR(AAUnderlyingObjects)
 PIPE_OPERATOR(AAAddressSpace)
+PIPE_OPERATOR(AAAllocationInfo)
 PIPE_OPERATOR(AAIndirectCallInfo)
 PIPE_OPERATOR(AAGlobalValueInfo)
 PIPE_OPERATOR(AADenormalFPMath)
@@ -881,11 +883,9 @@ struct AA::PointerInfo::State : public AbstractState {
                          AAPointerInfo::AccessKind Kind, Type *Ty,
                          Instruction *RemoteI = nullptr);
 
-  using OffsetBinsTy = DenseMap<RangeTy, SmallSet<unsigned, 4>>;
-
-  using const_bin_iterator = OffsetBinsTy::const_iterator;
-  const_bin_iterator begin() const { return OffsetBins.begin(); }
-  const_bin_iterator end() const { return OffsetBins.end(); }
+  AAPointerInfo::const_bin_iterator begin() const { return OffsetBins.begin(); }
+  AAPointerInfo::const_bin_iterator end() const { return OffsetBins.end(); }
+  int64_t numOffsetBins() const { return OffsetBins.size(); }
 
   const AAPointerInfo::Access &getAccess(unsigned Index) const {
     return AccessList[Index];
@@ -905,7 +905,7 @@ struct AA::PointerInfo::State : public AbstractState {
   // are all combined into a single Access object. This may result in loss of
   // information in RangeTy in the Access object.
   SmallVector<AAPointerInfo::Access> AccessList;
-  OffsetBinsTy OffsetBins;
+  AAPointerInfo::OffsetBinsTy OffsetBins;
   DenseMap<const Instruction *, SmallVector<unsigned>> RemoteIMap;
 
   /// See AAPointerInfo::forallInterferingAccesses.
@@ -1109,6 +1109,16 @@ struct AAPointerInfoImpl
     return AAPointerInfo::manifest(A);
   }
 
+  virtual const_bin_iterator begin() const override { return State::begin(); }
+  virtual const_bin_iterator end() const override { return State::end(); }
+  virtual int64_t numOffsetBins() const override {
+    return State::numOffsetBins();
+  }
+
+  virtual const Access &getBinAccess(unsigned Index) const override {
+    return getAccess(Index);
+  }
+
   bool forallInterferingAccesses(
       AA::RangeTy Range,
       function_ref<bool(const AAPointerInfo::Access &, bool)> CB)
@@ -1455,7 +1465,7 @@ struct AAPointerInfoImpl
   void trackPointerInfoStatistics(const IRPosition &IRP) const {}
 
   /// Dump the state into \p O.
-  void dumpState(raw_ostream &O) {
+  virtual void dumpState(raw_ostream &O) const override {
     for (auto &It : OffsetBins) {
       O << "[" << It.first.Offset << "-" << It.first.Offset + It.first.Size
         << "] : " << It.getSecond().size() << "\n";
@@ -6521,7 +6531,7 @@ struct AAValueSimplifyCallSiteReturned : AAValueSimplifyImpl {
 
   /// See AbstractAttribute::updateImpl(...).
   ChangeStatus updateImpl(Attributor &A) override {
-        return indicatePessimisticFixpoint();
+    return indicatePessimisticFixpoint();
   }
 
   void trackStatistics() const override {
@@ -12688,6 +12698,330 @@ struct AAAddressSpaceCallSiteArgument final : AAAddressSpaceImpl {
 };
 } // namespace
 
+/// ----------- Allocation Info ----------
+namespace {
+struct AAAllocationInfoImpl : public AAAllocationInfo {
+  AAAllocationInfoImpl(const IRPosition &IRP, Attributor &A)
+      : AAAllocationInfo(IRP, A) {}
+
+  std::optional<TypeSize> getAllocatedSize() const override {
+    assert(isValidState() && "the AA is invalid");
+    return AssumedAllocatedSize;
+  }
+
+  const NewOffsetsTy &getNewOffsets() const override {
+    assert(isValidState() && "the AA is invalid");
+    return NewComputedOffsets;
+  }
+
+  std::optional<TypeSize> findInitialAllocationSize(Instruction *I,
+                                                    const DataLayout &DL) {
+
+    // TODO: implement case for malloc like instructions
+    switch (I->getOpcode()) {
+    case Instruction::Alloca: {
+      AllocaInst *AI = cast<AllocaInst>(I);
+      return AI->getAllocationSize(DL);
+    }
+    default:
+      return std::nullopt;
+    }
+  }
+
+  ChangeStatus updateImpl(Attributor &A) override {
+
+    const IRPosition &IRP = getIRPosition();
+    Instruction *I = IRP.getCtxI();
+
+    // TODO: update check for malloc like calls
+    if (!isa<AllocaInst>(I))
+      return indicatePessimisticFixpoint();
+
+    bool IsKnownNoCapture;
+    if (!AA::hasAssumedIRAttr<Attribute::NoCapture>(
+            A, this, IRP, DepClassTy::OPTIONAL, IsKnownNoCapture))
+      return indicatePessimisticFixpoint();
+
+    const AAPointerInfo *PI =
+        A.getOrCreateAAFor<AAPointerInfo>(IRP, *this, DepClassTy::REQUIRED);
+
+    if (!PI)
+      return indicatePessimisticFixpoint();
+
+    if (!PI->getState().isValidState())
+      return indicatePessimisticFixpoint();
+
+    const DataLayout &DL = A.getDataLayout();
+    const auto AllocationSize = findInitialAllocationSize(I, DL);
+
+    // If allocation size is nullopt, we give up.
+    if (!AllocationSize)
+      return indicatePessimisticFixpoint();
+
+    // For zero sized allocations, we give up.
+    // Since we can't reduce further
+    if (*AllocationSize == 0)
+      return indicatePessimisticFixpoint();
+
+    int64_t NumBins = PI->numOffsetBins();
+
+    if (NumBins == 0) {
+      auto NewAllocationSize = std::optional<TypeSize>(TypeSize(0, false));
+      if (!changeAllocationSize(NewAllocationSize))
+        return ChangeStatus::UNCHANGED;
+      return ChangeStatus::CHANGED;
+    }
+
+    // For each bin, compute its new start Offset and store the results.
+    int64_t PrevBinEndOffset = 0;
+    bool ChangedOffsets = false;
+    for (auto It = PI->begin(); It != PI->end(); It++) {
+      const AA::RangeTy &OldRange = It->getFirst();
+
+      int64_t NewStartOffset = PrevBinEndOffset;
+      int64_t NewEndOffset = NewStartOffset + OldRange.Size;
+
+      PrevBinEndOffset = NewEndOffset;
+      ChangedOffsets |=
+          setOffets(OldRange, OldRange.Offset, NewStartOffset, OldRange.Size);
+    }
+
+    // Set the new size of the allocation, the new size if the New End Offset of
+    // the last Bin.
+    auto NewAllocationSize =
+        std::optional<TypeSize>(TypeSize(PrevBinEndOffset * 8, false));
+    if (!changeAllocationSize(NewAllocationSize))
+      return ChangeStatus::UNCHANGED;
+
+    if (!ChangedOffsets)
+      return ChangeStatus::UNCHANGED;
+
+    return ChangeStatus::CHANGED;
+  }
+
+  /// See AbstractAttribute::manifest(...).
+  ChangeStatus manifest(Attributor &A) override {
+
+    assert(isValidState() &&
+           "Manifest should only be called if the state is valid.");
+
+    const IRPosition &IRP = getIRPosition();
+    Instruction *I = IRP.getCtxI();
+
+    auto FixedAllocatedSizeInBits = getAllocatedSize()->getFixedValue();
+
+    unsigned long NumBytesToAllocate = (FixedAllocatedSizeInBits + 7) / 8;
+    bool Changed = false;
+    switch (I->getOpcode()) {
+    // TODO: add case for malloc like calls
+    case Instruction::Alloca: {
+
+      AllocaInst *AI = cast<AllocaInst>(I);
+
+      Type *CharType = Type::getInt8Ty(I->getContext());
+
+      auto *NumBytesToValue =
+          ConstantInt::get(I->getContext(), APInt(32, NumBytesToAllocate));
+
+      AllocaInst *NewAllocaInst =
+          new AllocaInst(CharType, AI->getAddressSpace(), NumBytesToValue,
+                         AI->getAlign(), AI->getName(), AI->getNextNode());
+
+      Changed |= A.changeAfterManifest(IRPosition::inst(*AI), *NewAllocaInst);
+
+      break;
+    }
+    default:
+      break;
+    }
+
+    const AAPointerInfo *PI =
+        A.getOrCreateAAFor<AAPointerInfo>(IRP, *this, DepClassTy::REQUIRED);
+
+    if (!PI)
+      return ChangeStatus::UNCHANGED;
+
+    if (!PI->getState().isValidState())
+      return ChangeStatus::UNCHANGED;
+
+    const auto &NewOffsets = getNewOffsets();
+    for (auto It = PI->begin(); It != PI->end(); It++) {
+
+      const auto &OldOffset = It->getFirst();
+
+      // If the OldOffsets are not present as a key, they did not change
+      // Hence, we should just continue
+      if (!NewOffsets.contains(OldOffset))
+        continue;
+
+      const auto &NewOffset = NewOffsets.lookup(OldOffset);
+      for (auto AccIndex : It->getSecond()) {
+
+        const auto &AccessInstruction = PI->getBinAccess(AccIndex);
+        const auto *LocalInst = AccessInstruction.getLocalInst();
+
+        auto UsePred = [&](const Use &U, bool &Follow) -> bool {
+          User *Usr = U.getUser();
+
+          auto NumOperands = LocalInst->getNumOperands();
+          for (unsigned Index = 0; Index < NumOperands; Index++) {
+            auto *Operand = LocalInst->getOperand(Index);
+            if (Operand == Usr) {
+              Instruction *UsrI = cast<Instruction>(Operand);
+              switch (UsrI->getOpcode()) {
+              case Instruction::GetElementPtr: {
+                // find the new offsets from the New OffsetMap, Change the
+                // instruction to use the new Offsets.
+                auto OffsetStart = NewOffset.Offset;
+                GetElementPtrInst *GEP = cast<GetElementPtrInst>(UsrI);
+                Value *NewComputedByteIndex =
+                    ConstantInt::get(I->getContext(), APInt(8, OffsetStart));
+                SmallVector<Value *, 1> ArrayRefVector{NewComputedByteIndex};
+                ArrayRef<Value *> AryRef = ArrayRef(ArrayRefVector);
+                GetElementPtrInst *NewGEP = GetElementPtrInst::Create(
+                    GEP->getPointerOperandType(), GEP->getPointerOperand(),
+                    AryRef, GEP->getName(), GEP->getNextNode());
+
+                Changed |=
+                    A.changeAfterManifest(IRPosition::inst(*GEP), *NewGEP);
+                break;
+              }
+              // TODO: Implement to fix load and store too.
+              default: {
+                break;
+              }
+              }
+            }
+          }
+          return true;
+        };
+
+        Changed |= A.checkForAllUses(UsePred, *this, IRP.getAssociatedValue());
+      }
+    }
+
+    if (!Changed)
+      return ChangeStatus::UNCHANGED;
+
+    return ChangeStatus::CHANGED;
+  }
+
+  /// See AbstractAttribute::getAsStr().
+  const std::string getAsStr(Attributor *A) const override {
+    if (!isValidState())
+      return "allocationinfo(<invalid>)";
+    return "allocationinfo(" +
+           (AssumedAllocatedSize == HasNoAllocationSize
+                ? "none"
+                : std::to_string(AssumedAllocatedSize->getFixedValue())) +
+           ")";
+  }
+
+  void dumpNewOffsetBins(raw_ostream &O) {
+
+    const auto &NewOffsetBins = getNewOffsets();
+    for (auto It = NewOffsetBins.begin(); It != NewOffsetBins.end(); It++) {
+
+      const auto &OldRange = It->getFirst();
+      const auto &NewRange = It->getSecond();
+
+      O << "[" << OldRange.Offset << "," << OldRange.Offset + OldRange.Size
+        << "] : ";
+      O << "[" << NewRange.Offset << "," << NewRange.Offset + NewRange.Size
+        << "]";
+      O << "\n\n";
+    }
+  }
+
+private:
+  std::optional<TypeSize> AssumedAllocatedSize = HasNoAllocationSize;
+  NewOffsetsTy NewComputedOffsets;
+
+  // Maintain the computed allocation size of the object.
+  // Returns (bool) weather the size of the allocation was modified or not.
+  bool changeAllocationSize(std::optional<TypeSize> Size) {
+    if (AssumedAllocatedSize == HasNoAllocationSize ||
+        AssumedAllocatedSize != Size) {
+      AssumedAllocatedSize = Size;
+      return true;
+    }
+    return false;
+  }
+
+  // Function to update the OldRange to the new Range.
+  bool setOffets(const AA::RangeTy &OldRange, int64_t OldOffset,
+                 int64_t NewComputedOffset, int64_t Size) {
+
+    if (OldOffset == NewComputedOffset)
+      return false;
+
+    AA::RangeTy &NewRange = NewComputedOffsets.getOrInsertDefault(OldRange);
+    NewRange.Offset = NewComputedOffset;
+    NewRange.Size = Size;
+
+    return true;
+  }
+};
+
+struct AAAllocationInfoFloating : AAAllocationInfoImpl {
+  AAAllocationInfoFloating(const IRPosition &IRP, Attributor &A)
+      : AAAllocationInfoImpl(IRP, A) {}
+
+  void trackStatistics() const override {
+    STATS_DECLTRACK_FLOATING_ATTR(allocationinfo);
+  }
+};
+
+struct AAAllocationInfoReturned : AAAllocationInfoImpl {
+  AAAllocationInfoReturned(const IRPosition &IRP, Attributor &A)
+      : AAAllocationInfoImpl(IRP, A) {}
+
+  /// See AbstractAttribute::initialize(...).
+  void initialize(Attributor &A) override {
+    // TODO: we don't rewrite function argument for now because it will need to
+    // rewrite the function signature and all call sites
+    (void)indicatePessimisticFixpoint();
+  }
+
+  void trackStatistics() const override {
+    STATS_DECLTRACK_FNRET_ATTR(allocationinfo);
+  }
+};
+
+struct AAAllocationInfoCallSiteReturned : AAAllocationInfoImpl {
+  AAAllocationInfoCallSiteReturned(const IRPosition &IRP, Attributor &A)
+      : AAAllocationInfoImpl(IRP, A) {}
+
+  void trackStatistics() const override {
+    STATS_DECLTRACK_CSRET_ATTR(allocationinfo);
+  }
+};
+
+struct AAAllocationInfoArgument : AAAllocationInfoImpl {
+  AAAllocationInfoArgument(const IRPosition &IRP, Attributor &A)
+      : AAAllocationInfoImpl(IRP, A) {}
+
+  void trackStatistics() const override {
+    STATS_DECLTRACK_ARG_ATTR(allocationinfo);
+  }
+};
+
+struct AAAllocationInfoCallSiteArgument : AAAllocationInfoImpl {
+  AAAllocationInfoCallSiteArgument(const IRPosition &IRP, Attributor &A)
+      : AAAllocationInfoImpl(IRP, A) {}
+
+  /// See AbstractAttribute::initialize(...).
+  void initialize(Attributor &A) override {
+
+    (void)indicatePessimisticFixpoint();
+  }
+
+  void trackStatistics() const override {
+    STATS_DECLTRACK_CSARG_ATTR(allocationinfo);
+  }
+};
+} // namespace
+
 const char AANoUnwind::ID = 0;
 const char AANoSync::ID = 0;
 const char AANoFree::ID = 0;
@@ -12721,6 +13055,7 @@ const char AAPointerInfo::ID = 0;
 const char AAAssumptionInfo::ID = 0;
 const char AAUnderlyingObjects::ID = 0;
 const char AAAddressSpace::ID = 0;
+const char AAAllocationInfo::ID = 0;
 const char AAIndirectCallInfo::ID = 0;
 const char AAGlobalValueInfo::ID = 0;
 const char AADenormalFPMath::ID = 0;
@@ -12854,6 +13189,7 @@ CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AANoUndef)
 CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AANoFPClass)
 CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AAPointerInfo)
 CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AAAddressSpace)
+CREATE_VALUE_ABSTRACT_ATTRIBUTE_FOR_POSITION(AAAllocationInfo)
 
 CREATE_ALL_ABSTRACT_ATTRIBUTE_FOR_POSITION(AAValueSimplify)
 CREATE_ALL_ABSTRACT_ATTRIBUTE_FOR_POSITION(AAIsDead)
diff --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-02-01-ReturnAttrs.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-02-01-ReturnAttrs.ll
index 8254ab6230440f5..e6062da1d1457b9 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-02-01-ReturnAttrs.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/2008-02-01-ReturnAttrs.ll
@@ -9,8 +9,8 @@ define internal i32 @deref(ptr %x) nounwind {
 ; CG...
[truncated]

@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch from c036a1e to 29bf401 Compare November 12, 2023 02:42
@github-actions
Copy link

github-actions bot commented Nov 12, 2023

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions cpp,h -- llvm/include/llvm/Transforms/IPO/Attributor.h llvm/lib/Transforms/IPO/AttributorAttributes.cpp --diff_from_common_commit

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 15c748a95..f3f45abfb 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -6280,7 +6280,8 @@ public:
 
     // Merge two access paths into one.
     void mergeAccessPaths(const AccessPathSetTy *AccessPathsNew) const {
-      assert(AccessPathsNew != nullptr && "Expected Access Paths to be non null!");
+      assert(AccessPathsNew != nullptr &&
+             "Expected Access Paths to be non null!");
       for (auto *Path : *AccessPathsNew)
         if (!existsChain(Path))
           AccessPaths->insert(Path);
@@ -6291,8 +6292,9 @@ public:
       bool IsSame = true;
       if (AccessPaths->size() != AccessPathsR->size())
         return false;
-      
-      assert(AccessPathsR != nullptr && "Expected Access Paths to be non null!");
+
+      assert(AccessPathsR != nullptr &&
+             "Expected Access Paths to be non null!");
       for (auto *Path : *AccessPathsR) {
         if (!existsChain(Path))
           IsSame = false;
@@ -6302,7 +6304,8 @@ public:
 
     // Check if the chain exists in the AccessPathsSet.
     bool existsChain(const AccessPathTy *NewPath) const {
-      assert(NewPath != nullptr && AccessPaths != nullptr && "Expected Access Paths to be non null!");
+      assert(NewPath != nullptr && AccessPaths != nullptr &&
+             "Expected Access Paths to be non null!");
       for (auto *OldPath : *AccessPaths)
         if (OldPath && *OldPath == *NewPath)
           return true;
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 966125032..0674a42d9 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -13867,8 +13867,9 @@ struct AAAllocationInfoImpl : public AAAllocationInfo {
         Type *PointeeTy = OldLoadInst->getPointerOperandType();
         int64_t ShiftValue = OffsetNew - OffsetOld;
         Value *IndexList[1] = {ConstantInt::get(Int64TyInteger, ShiftValue)};
-        Value *GepToNewAddress = GetElementPtrInst::Create(
-            PointeeTy, PointerOperand, IndexList, "NewGep", OldLoadInst->getIterator());
+        Value *GepToNewAddress =
+            GetElementPtrInst::Create(PointeeTy, PointerOperand, IndexList,
+                                      "NewGep", OldLoadInst->getIterator());
 
         LoadInst *NewLoadInst = new LoadInst(
             OldLoadInst->getType(), GepToNewAddress, OldLoadInst->getName(),
@@ -13890,12 +13891,13 @@ struct AAAllocationInfoImpl : public AAAllocationInfo {
             cast<Instruction>(OldStoreInst->getPointerOperand());
         Type *PointeeTy = OldStoreInst->getPointerOperandType();
         Value *IndexList[1] = {ConstantInt::get(Int64TyInteger, ShiftValue)};
-        Value *GepToNewAddress = GetElementPtrInst::Create(
-            PointeeTy, PointerOperand, IndexList, "NewGep", OldStoreInst->getIterator());
+        Value *GepToNewAddress =
+            GetElementPtrInst::Create(PointeeTy, PointerOperand, IndexList,
+                                      "NewGep", OldStoreInst->getIterator());
 
-        StoreInst *NewStoreInst =
-            new StoreInst(OldStoreInst->getValueOperand(), GepToNewAddress,
-                          false, OldStoreInst->getAlign(), OldStoreInst->getIterator());
+        StoreInst *NewStoreInst = new StoreInst(
+            OldStoreInst->getValueOperand(), GepToNewAddress, false,
+            OldStoreInst->getAlign(), OldStoreInst->getIterator());
 
         Changed |= A.changeAfterManifest(IRPosition::inst(*OldStoreInst),
                                          *NewStoreInst);
@@ -13909,7 +13911,8 @@ struct AAAllocationInfoImpl : public AAAllocationInfo {
         Value *IndexList[1] = {ConstantInt::get(Int64TyInteger, OffsetNew)};
         Value *OldPointerOperand = OldGEP->getPointerOperand();
         Value *GepToNewAddress = GetElementPtrInst::Create(
-            NewAllocationType, OldPointerOperand, IndexList, "NewGep", OldGEP->getIterator());
+            NewAllocationType, OldPointerOperand, IndexList, "NewGep",
+            OldGEP->getIterator());
 
         Changed |=
             A.changeAfterManifest(IRPosition::inst(*OldGEP), *GepToNewAddress);

@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch 6 times, most recently from 7f60962 to ae1d871 Compare November 12, 2023 04:07
@jdoerfert jdoerfert self-requested a review November 22, 2023 17:56
@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch from ae1d871 to 04fdbe9 Compare December 14, 2023 18:53
@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch from 04fdbe9 to 83f44a8 Compare December 14, 2023 19:15
@vidsinghal vidsinghal changed the title [Attributor] Fix GEP offsets if multiple bins are present for an allocation. [Attributor] Fix Load/Store offsets when an allocation size is changed for multiple access bins. Dec 14, 2023
@vidsinghal vidsinghal changed the title [Attributor] Fix Load/Store offsets when an allocation size is changed for multiple access bins. [Attributor] Fix Load/Store offsets in case of multiple access bins when an allocation size is changed. Dec 14, 2023
@jdoerfert
Copy link
Member

Is this ready for review? The commit message states something about existing errors.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just some drive-by notes)

@vidsinghal
Copy link
Contributor Author

Is this ready for review? The commit message states something about existing errors.

Hello Johannes, these errors were supposed to be discussed for a potential fix.

@vidsinghal
Copy link
Contributor Author

(Just some drive-by notes)

Hi Nikita, thanks for reviewing!

@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch 2 times, most recently from f4bbc49 to 3ac2f16 Compare April 20, 2024 17:59
@vidsinghal vidsinghal requested a review from nikic April 20, 2024 18:01
@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch 4 times, most recently from da7e567 to 83986bd Compare May 24, 2024 23:19
@kevinsala
Copy link
Contributor

Ping for reviewers. Another reviewing pass or more feedback would be greatly appreciated!

CC @nikic @shiltian @jhuber6

@shiltian
Copy link
Contributor

Can anyone rebase this PR?

@vidsinghal
Copy link
Contributor Author

Can anyone rebase this PR?

yes will do

@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch from ea992a1 to af41abc Compare October 25, 2025 02:40
@vidsinghal
Copy link
Contributor Author

vidsinghal commented Oct 25, 2025

Can anyone rebase this PR?

i have rebased this with the latest main

@github-actions
Copy link

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:
git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef([^a-zA-Z0-9_-]|$)|UndefValue::get)' 'HEAD~1' HEAD llvm/test/Transforms/Attributor/pointer-info-track-access-chain.ll llvm/include/llvm/Transforms/IPO/Attributor.h llvm/lib/Transforms/IPO/AttributorAttributes.cpp llvm/test/Transforms/Attributor/ArgumentPromotion/crash.ll llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead.ll llvm/test/Transforms/Attributor/ArgumentPromotion/live_called_from_dead_2.ll llvm/test/Transforms/Attributor/ArgumentPromotion/nonzero-address-spaces.ll llvm/test/Transforms/Attributor/IPConstantProp/return-argument.ll llvm/test/Transforms/Attributor/allocator.ll llvm/test/Transforms/Attributor/call-simplify-pointer-info.ll llvm/test/Transforms/Attributor/heap_to_stack.ll llvm/test/Transforms/Attributor/heap_to_stack_gpu.ll llvm/test/Transforms/Attributor/liveness.ll llvm/test/Transforms/Attributor/nodelete.ll llvm/test/Transforms/Attributor/pointer-info.ll llvm/test/Transforms/Attributor/reduced/openmp_opt_dont_follow_gep_without_value.ll llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll

The following files introduce new uses of undef:

  • llvm/test/Transforms/Attributor/reduced/openmp_opt_dont_follow_gep_without_value.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

@vidsinghal vidsinghal force-pushed the vidhush/aaallocationinfo_multibin branch from 23c8e37 to f1992d7 Compare October 25, 2025 21:52
@vidsinghal vidsinghal requested a review from shiltian October 26, 2025 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants