[PATCH] D119696: [AMDGPU] Improve v_cmpx usage on GFX10.3.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:896
   // V_NOP will be discarded by SQ.
-  // Use V_MOB_B32 v?, v?. Register must be alive so use src0 of V_PERMLANE*
+  // Use V_MOV_B32 v?, v?. Register must be alive so use src0 of V_PERMLANE*
   // which is always a VGPR and available.
----------------
Commit this separately, no review required.


================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:628
+                findPossibleVCMPVCMPXOptimization(MI, Exec, TRI, TII, *MRI))
+          SaveExecVCmpMapping[&MI] = &*VCmp;
+      }
----------------
Don't need `&*` here.


================
Comment at: llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp:737
       if (TII->isVOPC(Op32)) {
-        Register DstReg = MI.getOperand(0).getReg();
+        // Exclude VOPCX instructions as these don't write explicitly write a
+        // dst.
----------------
Typo "write explicitly write"


================
Comment at: llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp:741
+        if (!Op0.isReg() || !Op0.isDef())
+          continue;
+
----------------
"continue" doesn't seem right. We *should* be shrinking cmpx instructions from VOP3 encoding to VOPC encoding.


================
Comment at: llvm/lib/Target/AMDGPU/SIShrinkInstructions.cpp:753
           // this pass again after RA and shrink it if it outputs to VCC.
-          MRI.setRegAllocationHint(MI.getOperand(0).getReg(), 0, VCCReg);
+          MRI.setRegAllocationHint(Op0.getReg(), 0, VCCReg);
           continue;
----------------
Use "DstReg"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119696