[PATCH] D96386: [AMDGPU] Fix promote alloca with double use in a same insn

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:1004
+    case Intrinsic::memmove:
+      // These have 2 pointer operands. In case if second pointer also needs
+      // to be replaced we defer processing of these intrinsics until all
----------------
arsenm wrote:
> In principle this could happen for all multi-operand instructions, like select and icmp
We have tests for this. It works fine. Operands are just updated with replaceAllUsesWith(). With memcpy and memmove there are two issues:

1. We need to replace called function with different mangling. We are only replacing it for one of the arguments. Second argument is also updated but we do not visit this intrinsic again.
2. Even if we have these calls in the worklist twice that still would not work. Other instructions just get their operands updated, but here we are replacing call instruction itself with a new one. No pointer in the worklist would point to it.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:1007
+      // other values are processed.
+      DeferredIntrs.push_back(Intr);
       continue;
----------------
arsenm wrote:
> I'm not sure I understand why it really needs to be deferred. If we tracked the specific use, you could just replace the one operand and then encounter the instruction again for the second?
No, because instruction is dropped.


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

https://reviews.llvm.org/D96386