[PATCH] D100939: [ExpandPostRAPseudos] Don't add duplicate implicit operands

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D100939#2705349 <https://reviews.llvm.org/D100939#2705349>, @arsenm wrote:

> In D100939#2705343 <https://reviews.llvm.org/D100939#2705343>, @rampitec wrote:
>
>> In D100939#2704630 <https://reviews.llvm.org/D100939#2704630>, @foad wrote:
>>
>>> For AMDGPU should this be fixed earlier, by not putting the "implicit $exec" on a COPY? I don't understand whether they are required or not.
>>
>> It was done to prevent rescheduling VGPR COPYs across EXEC modifications. Still needed I believe.
>
> I don't think the way we throw implicit exec uses on COPYs at certain points is entirely sound. It's not applied consistently (and would really need to be present on copy creation)

I guess it would be reasonable to add all the implicit exec uses at the point when control flow is lowered to exec manipulation. Any instructions created after that would need the implicit use at creation. I don't know how close we are to achieving that.



================
Comment at: llvm/lib/CodeGen/ExpandPostRAPseudos.cpp:72-74
+    if (none_of(CopyMI->implicit_operands(), [&](const MachineOperand &CopyMO) {
+          return CopyMO.isIdenticalTo(MO);
+        }))
----------------
arsenm wrote:
> This feels like a hack and we should have avoided this
Not sure what you mean by "should have avoided" it. Are you saying the COPY should not have had the implicit exec use in the first place, or the MOV generated by copyPhysReg should not have had it, or TransferImplicitOperands should not exist?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100939