[PATCH] D53496: AMDGPU: Rewrite SILowerI1Copies to always stay on SALU

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIFixSGPRCopies.cpp:186
                              const SIRegisterInfo &TRI) {
-  return TRI.isSGPRClass(DstRC) && TRI.hasVGPRs(SrcRC);
+  return SrcRC != &AMDGPU::VReg_1RegClass && TRI.isSGPRClass(DstRC) &&
+         TRI.hasVGPRs(SrcRC);
----------------
Could we avoid still having VReg_1 by checking for i1 source instructions?


================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:88-90
+    if (Reg == AMDGPU::VCC || Reg == AMDGPU::EXEC)
+      return true;
+
----------------
Should this worry about sub registers? Can't this just use TRI.getRegClass instead of worrying about physical registers itself


================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:95-96
+    const TargetRegisterClass *RC = MRI->getRegClass(Reg);
+    return RC == &AMDGPU::SReg_64RegClass ||
+           RC == &AMDGPU::SReg_64_XEXECRegClass;
+  }
----------------
Could also be SGPR_64? I would prefer avoiding listing the register classes in another place


================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:164-175
+      // If this block has a divergent terminator and the def block is its
+      // post-dominator, the wave may first visit the other successors.
+      bool Divergent = false;
+      for (MachineInstr &MI : MBB->terminators()) {
+        if (MI.getOpcode() == AMDGPU::SI_NON_UNIFORM_BRCOND_PSEUDO ||
+            MI.getOpcode() == AMDGPU::SI_IF ||
+            MI.getOpcode() == AMDGPU::SI_ELSE ||
----------------
Separate function?


================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:513-514
+    for (MachineInstr &MI : MBB.phis()) {
+      if (!MI.isPHI())
+        continue;
+
----------------
Why is this necessary with phis()


================
Comment at: lib/Target/AMDGPU/SILowerI1Copies.cpp:771
+  if (PrevConstant && !PrevVal) {
+    BuildMI(MBB, I, DL, TII->get(AMDGPU::COPY), DstReg).addReg(CurMaskedReg);
+  } else if (CurConstant && !CurVal) {
----------------
.addRegs on new line


Repository:
  rL LLVM

https://reviews.llvm.org/D53496