-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[memprof] Migrate away from PointerUnion::is (NFC) #122622
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
[memprof] Migrate away from PointerUnion::is (NFC) #122622
Conversation
Note that PointerUnion::is have been soft deprecated in PointerUnion.h: // FIXME: Replace the uses of is(), get() and dyn_cast() with // isa<T>, cast<T> and the llvm::dyn_cast<T> In this patch, I'm calling call().getBase() for an instance of PointerUnion. call() alone would return an instance of IndexCall, which wraps PointerUnion. Note that isa<> cannot directly accept an instance of IndexCall, at least without defining CastInfo. I'm not touching PointerUnion::dyn_cast for now because it's a bit complicated; we could blindly migrate it to dyn_cast_if_present, but we should probably use dyn_cast when the operand is known to be non-null.
@llvm/pr-subscribers-llvm-transforms Author: Kazu Hirata (kazutakahirata) ChangesNote that PointerUnion::is have been soft deprecated in // FIXME: Replace the uses of is(), get() and dyn_cast() with In this patch, I'm calling call().getBase() for an instance of I'm not touching PointerUnion::dyn_cast for now because it's a bit Full diff: https://github.com/llvm/llvm-project/pull/122622.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
index 3d0e5cb5f97b4f..61a8f4a448bbd7 100644
--- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
+++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
@@ -3610,7 +3610,7 @@ IndexCallsiteContextGraph::cloneFunctionForCallsite(
// Confirm this matches the CloneNo provided by the caller, which is based on
// the number of function clones we have.
assert(CloneNo ==
- (Call.call().is<AllocInfo *>()
+ (isa<AllocInfo *>(Call.call().getBase())
? Call.call().dyn_cast<AllocInfo *>()->Versions.size()
: Call.call().dyn_cast<CallsiteInfo *>()->Clones.size()));
// Walk all the instructions in this function. Create a new version for
|
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 for now, though in this case it probably makes sense to add some helper methods to IndexCall.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11848 Here is the relevant piece of the build log for the reference
|
Note that PointerUnion::is have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa, cast and the llvm::dyn_cast
In this patch, I'm calling call().getBase() for an instance of
PointerUnion. call() alone would return an instance of IndexCall,
which wraps PointerUnion. Note that isa<> cannot directly accept an
instance of IndexCall, at least without defining CastInfo.
I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.