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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: lib/Target/AMDGPU/VOP3Instructions.td:399
+let Predicates = [isVI] in {
+def : AMDGPUPat <
+ (shl i64:$x, i32:$y),
----------------
alex-t wrote:
> 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);**
>       }
Then what the code "def V_LSHLREV_B64 : VOP3Inst <"v_lshlrev_b64", VOP_PAT_GEN<VOP3_Profile<VOP_I64_I32_I64>>, shl>" 8 lines above does? Isn't it the same?


================
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);
----------------
alex-t wrote:
> 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.
> 
How can it be possible for moveToVALU to create a VOP3 instruction without a VOP3 flag?
VOP3_Pseudo sets VOP3 flag. InstSI copies it into TSFlags. VOP3_Real copies TSFlags from a pseudo.


https://reviews.llvm.org/D52559