[llvm] 311fafd - [BasicAA] Fix -basicaa-recphi for geps with negative offsets
David Green via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 16 09:23:07 PDT 2020
Author: David Green
Date: 2020-07-16T17:22:40+01:00
New Revision: 311fafd2c90aed5b3fed9566503eebe629f1e979
URL: https://github.com/llvm/llvm-project/commit/311fafd2c90aed5b3fed9566503eebe629f1e979
DIFF: https://github.com/llvm/llvm-project/commit/311fafd2c90aed5b3fed9566503eebe629f1e979.diff
LOG: [BasicAA] Fix -basicaa-recphi for geps with negative offsets
As shown in D82998, the basic-aa-recphi option can cause miscompiles for
gep's with negative constants. The option checks for recursive phi, that
recurse through a contant gep. If it finds one, it performs aliasing
calculations using the other phi operands with an unknown size, to
specify that an unknown number of elements after the initial value are
potentially accessed. This works fine expect where the constant is
negative, as the size is still considered to be positive. So this patch
expands the check to make sure that the constant is also positive.
Differential Revision: https://reviews.llvm.org/D83576
Added:
Modified:
llvm/lib/Analysis/BasicAliasAnalysis.cpp
llvm/test/Analysis/BasicAA/recphi.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 74664098ce1d..33f122728d2a 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1648,8 +1648,32 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
}
SmallVector<Value *, 4> V1Srcs;
+ // For a recursive phi, that recurses through a contant gep, we can perform
+ // aliasing calculations using the other phi operands with an unknown size to
+ // specify that an unknown number of elements after the initial value are
+ // potentially accessed.
bool isRecursive = false;
- if (PV) {
+ auto CheckForRecPhi = [&](Value *PV) {
+ if (!EnableRecPhiAnalysis)
+ return false;
+ if (GEPOperator *PVGEP = dyn_cast<GEPOperator>(PV)) {
+ // Check whether the incoming value is a GEP that advances the pointer
+ // result of this PHI node (e.g. in a loop). If this is the case, we
+ // would recurse and always get a MayAlias. Handle this case specially
+ // below. We need to ensure that the phi is inbounds and has a constant
+ // positive operand so that we can check for alias with the initial value
+ // and an unknown but positive size.
+ if (PVGEP->getPointerOperand() == PN && PVGEP->isInBounds() &&
+ PVGEP->getNumIndices() == 1 && isa<ConstantInt>(PVGEP->idx_begin()) &&
+ !cast<ConstantInt>(PVGEP->idx_begin())->isNegative()) {
+ isRecursive = true;
+ return true;
+ }
+ }
+ return false;
+ };
+
+ if (PV) {
// If we have PhiValues then use it to get the underlying phi values.
const PhiValues::ValueSet &PhiValueSet = PV->getValuesForPhi(PN);
// If we have more phi values than the search depth then return MayAlias
@@ -1660,19 +1684,8 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
return MayAlias;
// Add the values to V1Srcs
for (Value *PV1 : PhiValueSet) {
- if (EnableRecPhiAnalysis) {
- if (GEPOperator *PV1GEP = dyn_cast<GEPOperator>(PV1)) {
- // Check whether the incoming value is a GEP that advances the pointer
- // result of this PHI node (e.g. in a loop). If this is the case, we
- // would recurse and always get a MayAlias. Handle this case specially
- // below.
- if (PV1GEP->getPointerOperand() == PN && PV1GEP->getNumIndices() == 1 &&
- isa<ConstantInt>(PV1GEP->idx_begin())) {
- isRecursive = true;
- continue;
- }
- }
- }
+ if (CheckForRecPhi(PV1))
+ continue;
V1Srcs.push_back(PV1);
}
} else {
@@ -1687,18 +1700,8 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
// and 'n' are the number of PHI sources.
return MayAlias;
- if (EnableRecPhiAnalysis)
- if (GEPOperator *PV1GEP = dyn_cast<GEPOperator>(PV1)) {
- // Check whether the incoming value is a GEP that advances the pointer
- // result of this PHI node (e.g. in a loop). If this is the case, we
- // would recurse and always get a MayAlias. Handle this case specially
- // below.
- if (PV1GEP->getPointerOperand() == PN && PV1GEP->getNumIndices() == 1 &&
- isa<ConstantInt>(PV1GEP->idx_begin())) {
- isRecursive = true;
- continue;
- }
- }
+ if (CheckForRecPhi(PV1))
+ continue;
if (UniqueSrc.insert(PV1).second)
V1Srcs.push_back(PV1);
diff --git a/llvm/test/Analysis/BasicAA/recphi.ll b/llvm/test/Analysis/BasicAA/recphi.ll
index 3c8397cc0fec..dfc88937bf69 100644
--- a/llvm/test/Analysis/BasicAA/recphi.ll
+++ b/llvm/test/Analysis/BasicAA/recphi.ll
@@ -92,8 +92,8 @@ if.end: ; preds = %f.exit
; CHECK: NoAlias: i32* %arrayidx1, i8* %0
; CHECK: NoAlias: i32* %arrayidx, i32* %arrayidx1
; CHECK: MayAlias: [10 x i32]* %tab, i32* %p.addr.05.i
-; CHECK: NoAlias: i32* %p.addr.05.i, i8* %0
-; CHECK: NoAlias: i32* %arrayidx, i32* %p.addr.05.i
+; CHECK: MayAlias: i32* %p.addr.05.i, i8* %0
+; CHECK: MayAlias: i32* %arrayidx, i32* %p.addr.05.i
; CHECK: MayAlias: i32* %arrayidx1, i32* %p.addr.05.i
; CHECK: MayAlias: [10 x i32]* %tab, i32* %incdec.ptr.i
; CHECK: MayAlias: i32* %incdec.ptr.i, i8* %0
@@ -141,17 +141,17 @@ if.end: ; preds = %f.exit
; CHECK: NoAlias: [3 x i16]* %int_arr.10, i16** %argv.6.par
; CHECK: NoAlias: i16* %_tmp1, i16** %argv.6.par
; CHECK: PartialAlias: [3 x i16]* %int_arr.10, i16* %_tmp1
-; CHECK: NoAlias: i16* %ls1.9.0, i16** %argv.6.par
+; CHECK: MayAlias: i16* %ls1.9.0, i16** %argv.6.par
; CHECK: MayAlias: [3 x i16]* %int_arr.10, i16* %ls1.9.0
; CHECK: MayAlias: i16* %_tmp1, i16* %ls1.9.0
-; CHECK: NoAlias: i16* %_tmp7, i16** %argv.6.par
+; CHECK: MayAlias: i16* %_tmp7, i16** %argv.6.par
; CHECK: MayAlias: [3 x i16]* %int_arr.10, i16* %_tmp7
; CHECK: MayAlias: i16* %_tmp1, i16* %_tmp7
; CHECK: NoAlias: i16* %_tmp7, i16* %ls1.9.0
; CHECK: NoAlias: i16* %_tmp11, i16** %argv.6.par
; CHECK: PartialAlias: [3 x i16]* %int_arr.10, i16* %_tmp11
; CHECK: NoAlias: i16* %_tmp1, i16* %_tmp11
-; CHECK: NoAlias: i16* %_tmp11, i16* %ls1.9.0
+; CHECK: MayAlias: i16* %_tmp11, i16* %ls1.9.0
; CHECK: MayAlias: i16* %_tmp11, i16* %_tmp7
; CHECK: Both ModRef: Ptr: i16** %argv.6.par <-> %_tmp16 = call i16 @call(i32 %_tmp13)
; CHECK: NoModRef: Ptr: [3 x i16]* %int_arr.10 <-> %_tmp16 = call i16 @call(i32 %_tmp13)
More information about the llvm-commits
mailing list