[llvm] 119a94b - [Attributor][FIX] Ensure to cache all intra procedural AA queries
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 17 18:01:18 PDT 2023
Author: Johannes Doerfert
Date: 2023-04-17T18:01:04-07:00
New Revision: 119a94b5aecb3c131381ff18fca0c2155dc42cec
URL: https://github.com/llvm/llvm-project/commit/119a94b5aecb3c131381ff18fca0c2155dc42cec
DIFF: https://github.com/llvm/llvm-project/commit/119a94b5aecb3c131381ff18fca0c2155dc42cec.diff
LOG: [Attributor][FIX] Ensure to cache all intra procedural AA queries
We failed to cache queries without an exclusion set that resulted in
non-reachable results. That is obviously bad as changes to liveness can
influence the result.
Fixes: https://github.com/llvm/llvm-project/issues/61883
Added:
llvm/test/Transforms/Attributor/reduced/missed_cached_entry_for_intra_reachability.ll
Modified:
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/test/Transforms/Attributor/depgraph.ll
llvm/test/Transforms/Attributor/noalias.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 9da926ec5eb35..7e1adfeece432 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -3582,9 +3582,10 @@ struct CachedReachabilityAA : public BaseTy {
if (!InUpdate)
QueryCache.erase(&RQI);
- // Insert a plain RQI (w/o exclusion set) if that makes sense.
- if (RQI.ExclusionSet &&
- (!UsedExclusionSet || Result == RQITy::Reachable::Yes)) {
+ // Insert a plain RQI (w/o exclusion set) if that makes sense. Two options:
+ // 1) If it is reachable, it doesn't matter if we have an exclusion set for this query.
+ // 2) We did not use the exclusion set, potentially because there is none.
+ if (Result == RQITy::Reachable::Yes || !UsedExclusionSet) {
RQITy PlainRQI(RQI.From, RQI.To);
if (!QueryCache.count(&PlainRQI)) {
RQITy *RQIPtr = new (A.Allocator) RQITy(RQI.From, RQI.To);
@@ -3594,9 +3595,8 @@ struct CachedReachabilityAA : public BaseTy {
}
}
- // Check if we need to insert a new permanent RQI.
- if (!InUpdate && (UsedExclusionSet ||
- (Result == RQITy::Reachable::Yes && RQI.ExclusionSet))) {
+ // Check if we need to insert a new permanent RQI with the exclusion set.
+ if (!InUpdate && Result != RQITy::Reachable::Yes && UsedExclusionSet) {
assert((!RQI.ExclusionSet || !RQI.ExclusionSet->empty()) &&
"Did not expect empty set!");
RQITy *RQIPtr = new (A.Allocator)
diff --git a/llvm/test/Transforms/Attributor/depgraph.ll b/llvm/test/Transforms/Attributor/depgraph.ll
index 9e93690d80965..38b98aa9ea653 100644
--- a/llvm/test/Transforms/Attributor/depgraph.ll
+++ b/llvm/test/Transforms/Attributor/depgraph.ll
@@ -129,7 +129,7 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
; GRAPH-EMPTY:
; GRAPH-NEXT: [AAInterFnReachability] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state #queries(1)
; GRAPH-EMPTY:
-; GRAPH-NEXT: [AAIntraFnReachability] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state #queries(0)
+; GRAPH-NEXT: [AAIntraFnReachability] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn:checkAndAdvance [checkAndAdvance at -1]} with state #queries(1)
; GRAPH-EMPTY:
; GRAPH-NEXT: [AACallEdges] for CtxI ' %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs: [@-1]} with state CallEdges[0,1]
; GRAPH-EMPTY:
diff --git a/llvm/test/Transforms/Attributor/noalias.ll b/llvm/test/Transforms/Attributor/noalias.ll
index 29b758f85ff61..2b80348bc2995 100644
--- a/llvm/test/Transforms/Attributor/noalias.ll
+++ b/llvm/test/Transforms/Attributor/noalias.ll
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals
-; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=3 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=2 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
; TEST 1 - negative.
diff --git a/llvm/test/Transforms/Attributor/reduced/missed_cached_entry_for_intra_reachability.ll b/llvm/test/Transforms/Attributor/reduced/missed_cached_entry_for_intra_reachability.ll
new file mode 100644
index 0000000000000..8c7fb1e0fde65
--- /dev/null
+++ b/llvm/test/Transforms/Attributor/reduced/missed_cached_entry_for_intra_reachability.ll
@@ -0,0 +1,135 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes --check-attributes --check-globals
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=14 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
+; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC
+
+; We did not properly cache the intra procedural reachability queries which lead
+; to the removal of stores that are actually needed.
+
+; Reported as part of https://github.com/llvm/llvm-project/issues/61883
+
+ at random = external global i1, align 4
+
+;.
+; CHECK: @[[RANDOM:[a-zA-Z0-9_$"\\.-]+]] = external global i1, align 4
+;.
+define void @widget(ptr %arg1, float %arg2, i64 %idx1, i64 %idx2, i32 %limit) {
+; CHECK: Function Attrs: nofree norecurse nounwind
+; CHECK-LABEL: define {{[^@]+}}@widget
+; CHECK-SAME: (ptr nocapture nofree writeonly [[ARG1:%.*]], float [[ARG2:%.*]], i64 [[IDX1:%.*]], i64 [[IDX2:%.*]], i32 [[LIMIT:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT: bb:
+; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [128 x float], i32 0, align 4
+; CHECK-NEXT: [[ALLOCA3:%.*]] = alloca [81 x float], i32 0, align 4
+; CHECK-NEXT: store float 0.000000e+00, ptr [[ALLOCA3]], align 4
+; CHECK-NEXT: br label [[BB4:%.*]]
+; CHECK: bb4:
+; CHECK-NEXT: [[C1:%.*]] = load volatile i1, ptr @random, align 4
+; CHECK-NEXT: br i1 [[C1]], label [[BB5:%.*]], label [[BB7:%.*]]
+; CHECK: bb5:
+; CHECK-NEXT: store float [[ARG2]], ptr [[ALLOCA3]], align 4
+; CHECK-NEXT: br label [[BB4]]
+; CHECK: bb7:
+; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[ADD:%.*]], [[BB11:%.*]] ], [ 0, [[BB4]] ]
+; CHECK-NEXT: [[ICMP:%.*]] = icmp slt i32 [[PHI]], 5
+; CHECK-NEXT: br i1 [[ICMP]], label [[BB8:%.*]], label [[BB12:%.*]]
+; CHECK: bb8:
+; CHECK-NEXT: [[C2:%.*]] = load volatile i1, ptr @random, align 4
+; CHECK-NEXT: br i1 [[C2]], label [[BB9:%.*]], label [[BB11]]
+; CHECK: bb9:
+; CHECK-NEXT: [[SEXT:%.*]] = sext i32 [[PHI]] to i64
+; CHECK-NEXT: [[GETELEMENTPTR10:%.*]] = getelementptr [128 x float], ptr [[ALLOCA]], i64 0, i64 [[SEXT]]
+; CHECK-NEXT: store float 1.000000e+00, ptr [[GETELEMENTPTR10]], align 4
+; CHECK-NEXT: br label [[BB8]]
+; CHECK: bb11:
+; CHECK-NEXT: [[ADD]] = add i32 1, [[PHI]]
+; CHECK-NEXT: br label [[BB7]]
+; CHECK: bb12:
+; CHECK-NEXT: [[C3:%.*]] = load volatile i1, ptr @random, align 4
+; CHECK-NEXT: br i1 [[C3]], label [[BB13:%.*]], label [[BB19:%.*]]
+; CHECK: bb13:
+; CHECK-NEXT: [[LOAD:%.*]] = load float, ptr [[ALLOCA3]], align 4
+; CHECK-NEXT: br label [[BB14:%.*]]
+; CHECK: bb14:
+; CHECK-NEXT: [[PHI15:%.*]] = phi i32 [ 0, [[BB13]] ], [ 1, [[BB17:%.*]] ]
+; CHECK-NEXT: [[ICMP16:%.*]] = icmp slt i32 [[PHI15]], 1
+; CHECK-NEXT: br i1 [[ICMP16]], label [[BB17]], label [[BB19]]
+; CHECK: bb17:
+; CHECK-NEXT: [[C4:%.*]] = load volatile i1, ptr @random, align 4
+; CHECK-NEXT: br i1 [[C4]], label [[BB18:%.*]], label [[BB14]]
+; CHECK: bb18:
+; CHECK-NEXT: store float [[LOAD]], ptr [[ALLOCA]], align 4
+; CHECK-NEXT: br label [[BB17]]
+; CHECK: bb19:
+; CHECK-NEXT: [[SEXT20:%.*]] = sext i32 0 to i64
+; CHECK-NEXT: [[LOAD22:%.*]] = load float, ptr [[ALLOCA]], align 4
+; CHECK-NEXT: store float [[LOAD22]], ptr [[ARG1]], align 4
+; CHECK-NEXT: ret void
+;
+bb:
+ %alloca = alloca [128 x float], i32 0, align 4
+ %alloca3 = alloca [81 x float], i32 0, align 4
+; store is removed
+ store float 0.0e+00, ptr %alloca3, align 4
+ br label %bb4
+
+bb4: ; preds = %bb5, %bb
+ %c1 = load volatile i1, ptr @random
+ br i1 %c1, label %bb5, label %bb7
+
+bb5: ; preds = %bb4
+; store is removed
+ store float %arg2, ptr %alloca3, align 4
+ br label %bb4
+
+bb7: ; preds = %bb11, %bb4
+ %phi = phi i32 [ %add, %bb11 ], [ 0, %bb4 ]
+ %icmp = icmp slt i32 %phi, 5
+ br i1 %icmp, label %bb8, label %bb12
+
+bb8: ; preds = %bb9, %bb7
+ %c2 = load volatile i1, ptr @random
+ br i1 %c2, label %bb9, label %bb11
+
+bb9: ; preds = %bb8
+ %sext = sext i32 %phi to i64
+ %getelementptr10 = getelementptr [128 x float], ptr %alloca, i64 0, i64 %sext
+ store float 1.000000e+00, ptr %getelementptr10, align 4
+ br label %bb8
+
+bb11: ; preds = %bb8
+ %add = add i32 1, %phi
+ br label %bb7
+
+bb12: ; preds = %bb7
+ %c3 = load volatile i1, ptr @random
+ br i1 %c3, label %bb13, label %bb19
+
+bb13: ; preds = %bb12
+ %load = load float, ptr %alloca3, align 4
+ br label %bb14
+
+bb14: ; preds = %bb17, %bb13
+ %phi15 = phi i32 [ 0, %bb13 ], [ 1, %bb17 ]
+ %icmp16 = icmp slt i32 %phi15, 1
+ br i1 %icmp16, label %bb17, label %bb19
+
+bb17: ; preds = %bb18, %bb14
+ %c4 = load volatile i1, ptr @random
+ br i1 %c4, label %bb18, label %bb14
+
+bb18: ; preds = %bb17
+ store float %load, ptr %alloca, align 4
+ br label %bb17
+
+bb19: ; preds = %bb14, %bb12
+ %sext20 = sext i32 0 to i64
+ %load22 = load float, ptr %alloca, align 4
+; Loaded value could be either 0.0,1.0,%arg2, or undef.
+ store float %load22, ptr %arg1, align 4
+ ret void
+}
+;.
+; CHECK: attributes #[[ATTR0]] = { nofree norecurse nounwind }
+;.
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CGSCC: {{.*}}
+; TUNIT: {{.*}}
More information about the llvm-commits
mailing list