[PATCH] D68635: [AMDGPU] Come back patch for the 'Assign register class for cross block values according to the divergence.'

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:653
       }
       case AMDGPU::PHI: {
+        unsigned hasVGPRUses = 0;
----------------
Can you split this into a function?


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:654
       case AMDGPU::PHI: {
-        Register Reg = MI.getOperand(0).getReg();
-        if (!TRI->isSGPRClass(MRI.getRegClass(Reg)))
-          break;
-
-        // We don't need to fix the PHI if the common dominator of the
-        // two incoming blocks terminates with a uniform branch.
-        bool HasVGPROperand = phiHasVGPROperands(MI, MRI, TRI, TII);
-        if (MI.getNumExplicitOperands() == 5 && !HasVGPROperand) {
-          MachineBasicBlock *MBB0 = MI.getOperand(2).getMBB();
-          MachineBasicBlock *MBB1 = MI.getOperand(4).getMBB();
-
-          if (!predsHasDivergentTerminator(MBB0, TRI) &&
-              !predsHasDivergentTerminator(MBB1, TRI)) {
-            LLVM_DEBUG(dbgs()
-                       << "Not fixing PHI for uniform branch: " << MI << '\n');
+        unsigned hasVGPRUses = 0;
+        SetVector<const MachineInstr *> worklist;
----------------
This isn't a bool, so shouldn't start with has


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:664
+              if (UseMI->isCopy() &&
+                  Register::isPhysicalRegister(UseMI->getOperand(0).getReg()) &&
+                  !TRI->isSGPRReg(MRI, UseMI->getOperand(0).getReg())) {
----------------
.isPhysical()


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:680-693
+            unsigned OpNo = UseMI->getOperandNo(&Use);
+            const MCInstrDesc &Desc = TII->get(UseMI->getOpcode());
+            if (!Desc.isPseudo() && Desc.OpInfo &&
+                OpNo < Desc.getNumOperands() &&
+                Desc.OpInfo[OpNo].RegClass != -1) {
+              const TargetRegisterClass *OpRC =
+                  TRI->getRegClass(Desc.OpInfo[OpNo].RegClass);
----------------
This seems to be reproducing SIInstrInfo::getOpRegClass


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:702
+              const TargetRegisterClass *RC =
+                  Register::isVirtualRegister(SrcReg)
+                      ? MRI.getRegClass(SrcReg)
----------------
SrcReg.isVirtual()


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:3978
   // No VOP2 instructions support AGPRs.
-  if (Src0.isReg() && RI.isAGPR(MRI, Src0.getReg()))
+  if (Src0.isReg() && RI.hasAGPRs(Register::isVirtualRegister(Src0.getReg())
+                                      ? MRI.getRegClass(Src0.getReg())
----------------
rampitec wrote:
> What's wrong with isAGPR() call?
.isVirtual()


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

https://reviews.llvm.org/D68635