[PATCH] D103694: [AMDGPU] Simplify handleAddressTakenFunctions. NFC.

Madhur Amilkanthwar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 24 00:12:09 PDT 2021


madhur13490 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPropagateAttributes.cpp:244
+  for (CallGraphNode *CGN : Reachable) {
+    if (Function *F = CGN->getFunction())
+      Changed |= removeAttributes(F);
----------------
foad wrote:
> madhur13490 wrote:
> > Please move this check inside the function otherwise every caller will have to do this check.
> I disagree. "Remove attributes from F" is a simpler API than "remove attributes from F, except if F is null". I have changed removeAttributes to take a reference to make it more obvious that F must be a function.
> 
> Also, there is only one caller, and even before my patch there were two callers but only one of them had to cope with F possibly being null. This is caused by a quirk of the CallGraph which contains artificial nodes for external callers/callees, where getFunction returns null.
Yes, the quirks of CallGraph makes me worry about this. Please make sure it does not regress as I had to specifically check for `!F` as a result of failed AOMP smoke tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103694/new/

https://reviews.llvm.org/D103694



More information about the llvm-commits mailing list