[PATCH] D101847: [AMDGPU] Fix function pointer argument bug in AMDGPU Propagate Attributes pass.

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPropagateAttributes.cpp:252
         CallBase *CI = dyn_cast<CallBase>(I);
-        if (!CI)
+        if (!CI || CI->getCalledFunction() != &F)
           continue;
----------------
jweightman wrote:
> arsenm wrote:
> > What if the callee was a bitcast of the function? Can you add another testcase with a constexpr function pointer cast?
> I tried adding this to the LIT test:
> ```
>     %h_cast = bitcast i32* ()* @h to i8* ()*
>     %1 = call i8* %h_cast()
> ...
> define private i32* @h() #1 {
>     %1 = alloca i32
>     ret i32* %1
> }
> ```
> 
> and the pass does not propagate the attributes like one would probably want. However, this seems pretty orthogonal to the problem I was trying to fix (preventing the illegal callee change), because the "user" is an `llvm::BitCastInstr` so `CI` is always `nullptr` in that case. 
> 
> In order to handle bitcasts, I think we would need new logic to detect which bitcasted functions need to be replaced, a new data structure to track them in (similar to the existing `ToReplace`), and new logic to do that replacement — all of which would be on a separate logical branch from the existing code for handling call instructions! Unless you see an easier way, I think such a change is beyond the scope of what I was trying to do.
This example isn't a constant expression cast. If you run opt -S -instcombine on this, it should turn it into the constant expr cast form. In that case the user will still be CallInst, it's just CI->getCalledFunction() will be a constantexpr cast of the function. stripPointerCasts should get you the base function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101847