[llvm] 4761d29 - [Attributor][FIX] Sanitize queries to LVI and ScalarEvolution
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 10 14:49:34 PDT 2021
Author: Johannes Doerfert
Date: 2021-07-10T16:45:19-05:00
New Revision: 4761d29633ac7889329dc6ef966eab01c7b7903d
URL: https://github.com/llvm/llvm-project/commit/4761d29633ac7889329dc6ef966eab01c7b7903d
DIFF: https://github.com/llvm/llvm-project/commit/4761d29633ac7889329dc6ef966eab01c7b7903d.diff
LOG: [Attributor][FIX] Sanitize queries to LVI and ScalarEvolution
When we talk to outside analyse, e.g., LVI and ScalarEvolution, we need
to be careful with the query. The particular error occurred because we
folded a PHI node before the LVI query but the context location was now
not dominated by the value anymore. This is not supported by LVI so we
have to filter these situations before we query the outside analyses.
Added:
Modified:
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/test/Transforms/Attributor/value-simplify.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 4da578683fe0..11c1bcdeb338 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -7134,11 +7134,42 @@ struct AAValueConstantRangeImpl : AAValueConstantRange {
const_cast<Instruction *>(CtxI));
}
+ /// Return true if \p CtxI is valid for querying outside analyses.
+ /// This basically makes sure we do not ask intra-procedural analysis
+ /// about a context in the wrong function or a context that violates
+ /// dominance assumptions they might have. The \p AllowAACtxI flag indicates
+ /// if the original context of this AA is OK or should be considered invalid.
+ bool isValidCtxInstructionForOutsideAnalysis(Attributor &A,
+ const Instruction *CtxI,
+ bool AllowAACtxI) const {
+ if (!CtxI || (!AllowAACtxI && CtxI == getCtxI()))
+ return false;
+
+ // Our context might be in a
diff erent function, neither intra-procedural
+ // analysis (ScalarEvolution nor LazyValueInfo) can handle that.
+ if (!AA::isValidInScope(getAssociatedValue(), CtxI->getFunction()))
+ return false;
+
+ // If the context is not dominated by the value there are paths to the
+ // context that do not define the value. This cannot be handled by
+ // LazyValueInfo so we need to bail.
+ if (auto *I = dyn_cast<Instruction>(&getAssociatedValue())) {
+ InformationCache &InfoCache = A.getInfoCache();
+ const DominatorTree *DT =
+ InfoCache.getAnalysisResultForFunction<DominatorTreeAnalysis>(
+ *I->getFunction());
+ return DT && DT->dominates(I, CtxI);
+ }
+
+ return true;
+ }
+
/// See AAValueConstantRange::getKnownConstantRange(..).
ConstantRange
getKnownConstantRange(Attributor &A,
const Instruction *CtxI = nullptr) const override {
- if (!CtxI || CtxI == getCtxI())
+ if (!isValidCtxInstructionForOutsideAnalysis(A, CtxI,
+ /* AllowAACtxI */ false))
return getKnown();
ConstantRange LVIR = getConstantRangeFromLVI(A, CtxI);
@@ -7154,9 +7185,8 @@ struct AAValueConstantRangeImpl : AAValueConstantRange {
// We may be able to bound a variable range via assumptions in
// Attributor. ex.) If x is assumed to be in [1, 3] and y is known to
// evolve to x^2 + x, then we can say that y is in [2, 12].
-
- if (!CtxI || CtxI == getCtxI() ||
- !AA::isValidInScope(getAssociatedValue(), CtxI->getFunction()))
+ if (!isValidCtxInstructionForOutsideAnalysis(A, CtxI,
+ /* AllowAACtxI */ false))
return getAssumed();
ConstantRange LVIR = getConstantRangeFromLVI(A, CtxI);
diff --git a/llvm/test/Transforms/Attributor/value-simplify.ll b/llvm/test/Transforms/Attributor/value-simplify.ll
index 8640f7671e3d..2ad3ef204321 100644
--- a/llvm/test/Transforms/Attributor/value-simplify.ll
+++ b/llvm/test/Transforms/Attributor/value-simplify.ll
@@ -1224,15 +1224,19 @@ declare i8* @m()
define i32 @test(i1 %c) {
; CHECK-LABEL: define {{[^@]+}}@test
; CHECK-SAME: (i1 [[C:%.*]]) {
-; CHECK-NEXT: [[R:%.*]] = call i32 @ctx_test(i1 [[C]])
-; CHECK-NEXT: ret i32 [[R]]
-;
- %r = call i32 @ctx_test(i1 %c)
- ret i32 %r
+; CHECK-NEXT: [[R1:%.*]] = call i32 @ctx_test1(i1 [[C]])
+; CHECK-NEXT: [[R2:%.*]] = call i32 @ctx_test2(i1 [[C]]), !range [[RNG0:![0-9]+]]
+; CHECK-NEXT: [[ADD:%.*]] = add i32 [[R1]], [[R2]]
+; CHECK-NEXT: ret i32 [[ADD]]
+;
+ %r1 = call i32 @ctx_test1(i1 %c)
+ %r2 = call i32 @ctx_test2(i1 %c)
+ %add = add i32 %r1, %r2
+ ret i32 %add
}
-define internal i32 @ctx_test(i1 %c) {
-; CHECK-LABEL: define {{[^@]+}}@ctx_test
+define internal i32 @ctx_test1(i1 %c) {
+; CHECK-LABEL: define {{[^@]+}}@ctx_test1
; CHECK-SAME: (i1 [[C:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[JOIN:%.*]]
@@ -1259,6 +1263,36 @@ join:
ret i32 %ret
}
+define internal i32 @ctx_test2(i1 %c) {
+; CHECK-LABEL: define {{[^@]+}}@ctx_test2
+; CHECK-SAME: (i1 [[C:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[JOIN:%.*]]
+; CHECK: then:
+; CHECK-NEXT: [[M:%.*]] = tail call i8* @m()
+; CHECK-NEXT: [[I:%.*]] = ptrtoint i8* [[M]] to i32
+; CHECK-NEXT: br label [[JOIN]]
+; CHECK: join:
+; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[I]], [[THEN]] ], [ undef, [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[RET:%.*]] = lshr i32 [[PHI]], 1
+; CHECK-NEXT: ret i32 [[RET]]
+;
+entry:
+ br i1 %c, label %then, label %join
+
+then:
+ %m = tail call i8* @m()
+ %i = ptrtoint i8* %m to i32
+ br label %join
+
+join:
+ %phi = phi i32 [ %i, %then ], [ undef, %entry ]
+ %ret = lshr i32 %phi, 1
+ ret i32 %ret
+
+ uselistorder label %join, { 1, 0 }
+}
+
;.
; IS__TUNIT_OPM: attributes #[[ATTR0]] = { nofree nosync nounwind willreturn }
; IS__TUNIT_OPM: attributes #[[ATTR1]] = { nofree nosync nounwind readnone willreturn }
@@ -1298,3 +1332,5 @@ join:
; IS__CGSCC_NPM: attributes #[[ATTR7]] = { readonly willreturn }
; IS__CGSCC_NPM: attributes #[[ATTR8]] = { nofree nosync nounwind readnone }
;.
+; CHECK: [[RNG0]] = !{i32 0, i32 -2147483648}
+;.
More information about the llvm-commits
mailing list