[BasicAA] Fix caching in the presence of phi cycles

Any time we insert a block into VisitedPhiBBs, previously cached
values may no longer be valid for the recursive alias queries. As
such, perform them using an empty AAQueryInfo.

Note that if we recurse to the same phi, the block will already
be inserted, so we reuse the old AAQueryInfo, and thus still
protect against infinite recursion.

This problem can appear with with an without BatchAA, but is more
likely to occur with BatchAA, as more values are cached.

Differential Revision: https://reviews.llvm.org/D90066
This commit is contained in:
Nikita Popov 2020-10-23 20:50:05 +02:00
parent 7c026a83ee
commit d09c592142
2 changed files with 19 additions and 8 deletions

View File

@ -1655,14 +1655,20 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
// In the recursive alias queries below, we may compare values from two // In the recursive alias queries below, we may compare values from two
// different loop iterations. Keep track of visited phi blocks, which will // different loop iterations. Keep track of visited phi blocks, which will
// be used when determining value equivalence. // be used when determining value equivalence.
auto Pair = VisitedPhiBBs.insert(PN->getParent()); bool BlockInserted = VisitedPhiBBs.insert(PN->getParent()).second;
auto _ = make_scope_exit([&]() { auto _ = make_scope_exit([&]() {
if (Pair.second) if (BlockInserted)
VisitedPhiBBs.erase(PN->getParent()); VisitedPhiBBs.erase(PN->getParent());
}); });
// If we inserted a block into VisitedPhiBBs, alias analysis results that
// have been cached earlier may no longer be valid. Perform recursive queries
// with a new AAQueryInfo.
AAQueryInfo NewAAQI;
AAQueryInfo *UseAAQI = BlockInserted ? &NewAAQI : &AAQI;
AliasResult Alias = aliasCheck(V2, V2Size, V2AAInfo, V1Srcs[0], PNSize, AliasResult Alias = aliasCheck(V2, V2Size, V2AAInfo, V1Srcs[0], PNSize,
PNAAInfo, AAQI, UnderV2); PNAAInfo, *UseAAQI, UnderV2);
// Early exit if the check of the first PHI source against V2 is MayAlias. // Early exit if the check of the first PHI source against V2 is MayAlias.
// Other results are not possible. // Other results are not possible.
@ -1678,8 +1684,8 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
for (unsigned i = 1, e = V1Srcs.size(); i != e; ++i) { for (unsigned i = 1, e = V1Srcs.size(); i != e; ++i) {
Value *V = V1Srcs[i]; Value *V = V1Srcs[i];
AliasResult ThisAlias = AliasResult ThisAlias = aliasCheck(V2, V2Size, V2AAInfo, V, PNSize,
aliasCheck(V2, V2Size, V2AAInfo, V, PNSize, PNAAInfo, AAQI, UnderV2); PNAAInfo, *UseAAQI, UnderV2);
Alias = MergeAliasResults(ThisAlias, Alias); Alias = MergeAliasResults(ThisAlias, Alias);
if (Alias == MayAlias) if (Alias == MayAlias)
break; break;

View File

@ -249,12 +249,17 @@ TEST_F(AliasAnalysisTest, BatchAAPhiCycles) {
auto &AA = getAAResults(*F); auto &AA = getAAResults(*F);
EXPECT_EQ(NoAlias, AA.alias(A1Loc, A2Loc)); EXPECT_EQ(NoAlias, AA.alias(A1Loc, A2Loc));
EXPECT_EQ(MayAlias, AA.alias(PhiLoc, A1Loc)); EXPECT_EQ(MayAlias, AA.alias(PhiLoc, A1Loc));
EXPECT_EQ(NoAlias, AA.alias(S1Loc, S2Loc)); // TODO: This is wrong EXPECT_EQ(MayAlias, AA.alias(S1Loc, S2Loc));
BatchAAResults BatchAA(AA); BatchAAResults BatchAA(AA);
EXPECT_EQ(NoAlias, BatchAA.alias(A1Loc, A2Loc)); EXPECT_EQ(NoAlias, BatchAA.alias(A1Loc, A2Loc));
EXPECT_EQ(NoAlias, BatchAA.alias(PhiLoc, A1Loc)); // TODO: This is wrong. EXPECT_EQ(MayAlias, BatchAA.alias(PhiLoc, A1Loc));
EXPECT_EQ(NoAlias, BatchAA.alias(S1Loc, S2Loc)); // TODO: This is wrong EXPECT_EQ(MayAlias, BatchAA.alias(S1Loc, S2Loc));
BatchAAResults BatchAA2(AA);
EXPECT_EQ(NoAlias, BatchAA2.alias(A1Loc, A2Loc));
EXPECT_EQ(MayAlias, BatchAA2.alias(S1Loc, S2Loc));
EXPECT_EQ(MayAlias, BatchAA2.alias(PhiLoc, A1Loc));
} }
class AAPassInfraTest : public testing::Test { class AAPassInfraTest : public testing::Test {