[PATCH] D98884: [IR] Ignore bitcasts of function pointers which are only used as callees in callbase instruction
Stanislav Mekhanoshin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 29 11:14:02 PDT 2021
rampitec added a comment.
It looks fine to me with a style changes in couple places and positive checks in new tests.
================
Comment at: llvm/lib/IR/Function.cpp:1630
+ if (const CallBase *CB = dyn_cast<CallBase>(U.getUser()))
+ return CB->isCallee(&U);
+ return false;
----------------
foad wrote:
> This does not respect IgnoreCallbackUses, IgnoreAssumeLikeCalls, IgnoreLLVMUsed. Also it does not handle bitcasts-of-bitcasts.
>
> Wouldn't the whole function be better implemented as a worklist algorithm now?
> ```
> SmallVector<Use &> WorkList(use_begin(), use_end());
> while (!WorkList.empty()) {
> U = WorkList.pop_back_val();
> if (isa<BitCast>(U))
> WorkList.push_back(U.use_begin(), U.use_end());
> else if (isa<CallBase>(U))
> ... all the existing logic that handles Ignore* flags ...
> }
> ```
In general yes, but this patch is growing. This would be a good followup change.
The pass calls hasAddressTaken() without arguments so that is not exerciseable here as is. For the same reason I withdraw my request to split the patch. Also note this may change if this is landed: D96179.
================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:2137
+ CallBase *CB = dyn_cast<CallBase>(UU);
+ if (!CB)
+ return;
----------------
A readbility change would be nice:
```
if (!CB && !CB->isCallee(&U))
return
```
================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:2170
+ CallBase *CB = dyn_cast<CallBase>(U.getUser());
+ if (!CB)
+ return;
----------------
Same here: if (!CB && !CB->isCallee(&U))
================
Comment at: llvm/test/Transforms/GlobalOpt/assumelike-bitcast-fastcc.ll:13
+
+; CHECK-NOT: define internal fastcc void @bar() {
+define internal void @bar() {
----------------
You need to check something positive. You can just check it is 'define internal void @bar() {'.
================
Comment at: llvm/test/Transforms/GlobalOpt/bitcast-call-argument-fastcc.ll:9
+
+; CHECK-NOT: define internal fastcc i32 @foo() {
+define internal i32 @foo() {
----------------
Same here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98884/new/
https://reviews.llvm.org/D98884
More information about the llvm-commits
mailing list