[PATCH] D52559: [AMDGPU] Divergence driven instruction selection. Shift operations.

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: lib/Target/AMDGPU/VOP3Instructions.td:399
+let Predicates = [isVI] in {
+def : AMDGPUPat <
+ (shl i64:$x, i32:$y),
----------------
rampitec wrote:
> Why not GCNPat?
> Why divergence is not checked?
> Why do you need it at all if you have patgen enabled for these instructions above?
On VI+ we have no V_LSHL_B64 only V_LSHLREV_B64, same for sra, srl.
So, we cannot select in case we have, let's say, shl with src0 i64 and src1 i32.
The aim of these patterns to swap operands. Current implementation does this in 
SIInstrInfo::moveToVALU

    case AMDGPU::S_LSHL_B64:
      if (ST.getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS) {
        NewOpcode = AMDGPU::V_LSHLREV_B64;
        **swapOperands(Inst);**
      }


================
Comment at: lib/Target/AMDGPU/VOP3Instructions.td:587
+    VOP3e_si <op, !cast<VOP3_Pseudo>(NAME).Pfl> {
+      // Hack to stop printing _e64
+      VOP3_Pseudo ps = !cast<VOP3_Pseudo>(NAME);
----------------
rampitec wrote:
> Does it really belong to this patch?
Sure it does. As soon as I start selecting this _b64  the LIT tests failed because of the odd "_e64" suffix printed by AMDGPUInstrPrinter. It prints it for all with VOP3 flag.
The flag was not set before this change because the instructions were created in moveToVALU.  
I filed the bug https://bugs.llvm.org/show_bug.cgi?id=39086 for adding the flag to indicate that the instruction does not have 32bit encoding.



https://reviews.llvm.org/D52559