-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[AssumptionCache] Don't use ResultElem for assumption list (NFC) #160462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ResultElem stores a weak handle of an assume, plus an index for referring to a specific operand bundle. This makes sense for the results of assumptionsFor(), which refers to specific operands of assumes. However, assumptions() is a plain list of assumes. It does not contain separate entries for each operand bundles. The operand bundle index is always ExprResultIdx. As such, we should be directly using WeakVH for this case, without the additional wrapper.
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesResultElem stores a weak handle of an assume, plus an index for referring to a specific operand bundle. This makes sense for the results of assumptionsFor(), which refers to specific operands of assumes. However, assumptions() is a plain list of assumes. It does not contain separate entries for each operand bundles. The operand bundle index is always ExprResultIdx. As such, we should be directly using WeakVH for this case, without the additional wrapper. Full diff: https://github.com/llvm/llvm-project/pull/160462.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/AssumptionCache.h b/llvm/include/llvm/Analysis/AssumptionCache.h
index 1b026ef76a45e..be33be3bf2e87 100644
--- a/llvm/include/llvm/Analysis/AssumptionCache.h
+++ b/llvm/include/llvm/Analysis/AssumptionCache.h
@@ -65,7 +65,7 @@ class AssumptionCache {
/// Vector of weak value handles to calls of the \@llvm.assume
/// intrinsic.
- SmallVector<ResultElem, 4> AssumeHandles;
+ SmallVector<WeakVH, 4> AssumeHandles;
class LLVM_ABI AffectedValueCallbackVH final : public CallbackVH {
AssumptionCache *AC;
@@ -148,7 +148,7 @@ class AssumptionCache {
/// FIXME: We should replace this with pointee_iterator<filter_iterator<...>>
/// when we can write that to filter out the null values. Then caller code
/// will become simpler.
- MutableArrayRef<ResultElem> assumptions() {
+ MutableArrayRef<WeakVH> assumptions() {
if (!Scanned)
scanFunction();
return AssumeHandles;
diff --git a/llvm/lib/Analysis/AssumptionCache.cpp b/llvm/lib/Analysis/AssumptionCache.cpp
index 980a891266e50..45ff9161db97c 100644
--- a/llvm/lib/Analysis/AssumptionCache.cpp
+++ b/llvm/lib/Analysis/AssumptionCache.cpp
@@ -172,7 +172,7 @@ void AssumptionCache::scanFunction() {
for (BasicBlock &B : F)
for (Instruction &I : B)
if (isa<AssumeInst>(&I))
- AssumeHandles.push_back({&I, ExprResultIdx});
+ AssumeHandles.push_back(&I);
// Mark the scan as complete.
Scanned = true;
@@ -188,7 +188,7 @@ void AssumptionCache::registerAssumption(AssumeInst *CI) {
if (!Scanned)
return;
- AssumeHandles.push_back({CI, ExprResultIdx});
+ AssumeHandles.push_back(CI);
#ifndef NDEBUG
assert(CI->getParent() &&
diff --git a/llvm/lib/Transforms/Scalar/DropUnnecessaryAssumes.cpp b/llvm/lib/Transforms/Scalar/DropUnnecessaryAssumes.cpp
index c2e58ba393553..149b6f6f5b5ed 100644
--- a/llvm/lib/Transforms/Scalar/DropUnnecessaryAssumes.cpp
+++ b/llvm/lib/Transforms/Scalar/DropUnnecessaryAssumes.cpp
@@ -21,8 +21,8 @@ DropUnnecessaryAssumesPass::run(Function &F, FunctionAnalysisManager &FAM) {
AssumptionCache &AC = FAM.getResult<AssumptionAnalysis>(F);
bool Changed = false;
- for (AssumptionCache::ResultElem &Elem : AC.assumptions()) {
- auto *Assume = cast_or_null<AssumeInst>(Elem.Assume);
+ for (WeakVH &Elem : AC.assumptions()) {
+ auto *Assume = cast_or_null<AssumeInst>(Elem);
if (!Assume)
continue;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
|
||
for (AssumptionCache::ResultElem &Elem : AC.assumptions()) { | ||
auto *Assume = cast_or_null<AssumeInst>(Elem.Assume); | ||
for (WeakVH &Elem : AC.assumptions()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (WeakVH &Elem : AC.assumptions()) { | |
for (const WeakVH &Elem : AC.assumptions()) { |
ResultElem stores a weak handle of an assume, plus an index for referring to a specific operand bundle. This makes sense for the results of assumptionsFor(), which refers to specific operands of assumes.
However, assumptions() is a plain list of assumes. It does not contain separate entries for each operand bundles. The operand bundle index is always ExprResultIdx.
As such, we should be directly using WeakVH for this case, without the additional wrapper.