[PATCH] D113005: [AMDGPU] Fix subreg checks in S_MOV_B64_IMM_PSEUDO generation.

[Vang Thao via Phabricator via llvm-commits]

vangthao added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:102
+      unsigned Opc = I.getOpcode();
+      if (Opc == AMDGPU::COPY || Opc == AMDGPU::S_MOV_B32) {
+        switch (I.getOperand(0).getSubReg()) {
----------------
arsenm wrote:
> Checking for specific opcodes with specific subreg indices is definitely not the right way to check for register interference.
Why do we check for COPY here? If Reg is not AGPR then we will not touch any COPY instructions.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:119
+    if (NumDef0 > 1 || NumDef1 > 1)
+      return false;
+  }
----------------
Is there a reason why we need a new loop to check this?

This should do the same as the removed lines of code below:

>         if (Def0)
>           return false;

>         if (Def1)
>           return false;

As in if there are more than one def of a subreg then bail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113005