[PATCH] D98884: [IR] Ignore bitcasts of function pointers which are only used as callees in callbase instruction

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 25 11:22:13 PDT 2021


jdoerfert added a comment.

In D98884#2650715 <https://reviews.llvm.org/D98884#2650715>, @madhur13490 wrote:

> @jdoerfert This patch is blocking https://reviews.llvm.org/D99347 for AMDGPU. Please have a look.

I'm looking and it seems there is more content added every revision which makes it a moving target.

Also, if you don't get feedback, it is custom to wait for a week to ping: https://www.llvm.org/docs/Contributing.html#how-to-submit-a-patch



================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:2135-2139
+  CallBase *CB = dyn_cast<CallBase>(U);
+  if (!CB) {
+    assert(false && "Expected CallBase");
+    return;
+  }
----------------
arsenm wrote:
> Just use cast<>
This can at some point error out. Why would the user of `ChangeCalleesToFastCall` and others need to require this property?


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:2156
   }
 }
 
----------------
This part was not in the original patch and is now tagged along. At the very least add it to the commit message. It is also unclear why you chose these two functions, are there no other users() loops?


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:2185
+    }
+    SetCallBaseAttributes(U, F, A);
   }
----------------
The function is called remove... and the helper set...


================
Comment at: llvm/test/Analysis/CallGraph/ignore-bitcast-call-argument-callee.ll:12
+; CHECK-NEXT:  CS<{{.*}}> calls function 'bar'
+; CHECK-NEXT:  CS<{{.*}}> calls external node
+
----------------
I run this with trunk and the output looks the same. What exactly is this testing?


================
Comment at: llvm/test/Transforms/GlobalOpt/bitcast-callees-fastcc.ll:17
+  ret void
+}
+
----------------
Also unclear what is tested here? Looks like the trunk output.


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