[llvm] 81f3dee - [Attributor][FIX] Ensure to delete all AAs

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2022-12-17T17:45:25-08:00
New Revision: 81f3deec1931380189e7a5d4fc45cd9b9023f260

URL: https://github.com/llvm/llvm-project/commit/81f3deec1931380189e7a5d4fc45cd9b9023f260
DIFF: https://github.com/llvm/llvm-project/commit/81f3deec1931380189e7a5d4fc45cd9b9023f260.diff

LOG: [Attributor][FIX] Ensure to delete all AAs

Before we might have missed calling the destructor on an abstract
attribute if it was created outside the seeding or update phase.
All AAs are now in the AAMap and we can use it to delete them all.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/IPO/Attributor.h
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/allow_list.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 768e5ea2ec0e6..d311c399f8bc6 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -1484,14 +1484,16 @@ struct Attributor {
     // Use the static create method.
     auto &AA = AAType::createForPosition(IRP, *this);
 
+    // Always register a new attribute to make sure we clean up the allocated
+    // memory properly.
+    registerAA(AA);
+
     // If we are currenty seeding attributes, enforce seeding rules.
     if (Phase == AttributorPhase::SEEDING && !shouldSeedAttribute(AA)) {
       AA.getState().indicatePessimisticFixpoint();
       return AA;
     }
 
-    registerAA(AA);
-
     // For now we ignore naked and optnone functions.
     bool Invalidate =
         Configuration.Allowed && !Configuration.Allowed->count(&AAType::ID);

diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index a1bb3544ee8d9..b017961d170df 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1176,8 +1176,8 @@ std::optional<Value *> Attributor::translateArgumentToCallSiteContent(
 Attributor::~Attributor() {
   // The abstract attributes are allocated via the BumpPtrAllocator Allocator,
   // thus we cannot delete them. We can, and want to, destruct them though.
-  for (auto &DepAA : DG.SyntheticRoot.Deps) {
-    AbstractAttribute *AA = cast<AbstractAttribute>(DepAA.getPointer());
+  for (auto &It : AAMap) {
+    AbstractAttribute *AA = It.getSecond();
     AA->~AbstractAttribute();
   }
 }

diff  --git a/llvm/test/Transforms/Attributor/allow_list.ll b/llvm/test/Transforms/Attributor/allow_list.ll
index 31cb6bbc66281..528e730815bcb 100644
--- a/llvm/test/Transforms/Attributor/allow_list.ll
+++ b/llvm/test/Transforms/Attributor/allow_list.ll
@@ -35,10 +35,12 @@ define internal i32 @range_test(i32 %a) #0 {
 ; CHECK_DISABLED_FUNCTION-NEXT:    [[TMP2:%.*]] = zext i1 [[TMP1]] to i32
 ; CHECK_DISABLED_FUNCTION-NEXT:    ret i32 [[TMP2]]
 ;
-; CHECK_ENABLED_FUNCTION: Function Attrs: noinline nounwind memory(none) uwtable
+; CHECK_ENABLED_FUNCTION: Function Attrs: noinline nounwind uwtable
 ; CHECK_ENABLED_FUNCTION-LABEL: define {{[^@]+}}@range_test
-; CHECK_ENABLED_FUNCTION-SAME: () #[[ATTR0:[0-9]+]] {
-; CHECK_ENABLED_FUNCTION-NEXT:    ret i32 1
+; CHECK_ENABLED_FUNCTION-SAME: (i32 [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK_ENABLED_FUNCTION-NEXT:    [[TMP1:%.*]] = icmp sgt i32 [[A]], 100
+; CHECK_ENABLED_FUNCTION-NEXT:    [[TMP2:%.*]] = zext i1 [[TMP1]] to i32
+; CHECK_ENABLED_FUNCTION-NEXT:    ret i32 [[TMP2]]
 ;
   %1 = icmp sgt i32 %a, 100
   %2 = zext i1 %1 to i32
@@ -65,10 +67,11 @@ define i32 @range_use1() #0 {
 ; CHECK_DISABLED_FUNCTION-NEXT:    [[TMP1:%.*]] = call i32 @range_test(i32 123)
 ; CHECK_DISABLED_FUNCTION-NEXT:    ret i32 [[TMP1]]
 ;
-; CHECK_ENABLED_FUNCTION: Function Attrs: nofree noinline norecurse nosync nounwind willreturn memory(none) uwtable
+; CHECK_ENABLED_FUNCTION: Function Attrs: noinline norecurse nounwind uwtable
 ; CHECK_ENABLED_FUNCTION-LABEL: define {{[^@]+}}@range_use1
 ; CHECK_ENABLED_FUNCTION-SAME: () #[[ATTR1:[0-9]+]] {
-; CHECK_ENABLED_FUNCTION-NEXT:    ret i32 1
+; CHECK_ENABLED_FUNCTION-NEXT:    [[TMP1:%.*]] = call i32 @range_test(i32 noundef 123) #[[ATTR2:[0-9]+]]
+; CHECK_ENABLED_FUNCTION-NEXT:    ret i32 [[TMP1]]
 ;
   %1 = call i32 @range_test(i32 123)
   ret i32 %1
@@ -96,8 +99,8 @@ define i32 @range_use2() #0 {
 ;
 ; CHECK_ENABLED_FUNCTION: Function Attrs: noinline nounwind uwtable
 ; CHECK_ENABLED_FUNCTION-LABEL: define {{[^@]+}}@range_use2
-; CHECK_ENABLED_FUNCTION-SAME: () #[[ATTR2:[0-9]+]] {
-; CHECK_ENABLED_FUNCTION-NEXT:    [[TMP1:%.*]] = call i32 @range_test()
+; CHECK_ENABLED_FUNCTION-SAME: () #[[ATTR0]] {
+; CHECK_ENABLED_FUNCTION-NEXT:    [[TMP1:%.*]] = call i32 @range_test(i32 123)
 ; CHECK_ENABLED_FUNCTION-NEXT:    ret i32 [[TMP1]]
 ;
   %1 = call i32 @range_test(i32 123)
@@ -112,7 +115,7 @@ attributes #0 = { nounwind uwtable noinline }
 ;.
 ; CHECK_DISABLED_FUNCTION: attributes #[[ATTR0]] = { noinline nounwind uwtable }
 ;.
-; CHECK_ENABLED_FUNCTION: attributes #[[ATTR0]] = { noinline nounwind memory(none) uwtable }
-; CHECK_ENABLED_FUNCTION: attributes #[[ATTR1]] = { nofree noinline norecurse nosync nounwind willreturn memory(none) uwtable }
-; CHECK_ENABLED_FUNCTION: attributes #[[ATTR2]] = { noinline nounwind uwtable }
+; CHECK_ENABLED_FUNCTION: attributes #[[ATTR0]] = { noinline nounwind uwtable }
+; CHECK_ENABLED_FUNCTION: attributes #[[ATTR1]] = { noinline norecurse nounwind uwtable }
+; CHECK_ENABLED_FUNCTION: attributes #[[ATTR2]] = { nounwind }
 ;.