[AMDGPU] Enable `permlanex16` selection with `+16-bit-insts,+gfx10-insts`

[Pierre-vh]

Summary

ROCm device libs can emit permlanex w/ the +gfx10-insts attribute, and it counts on the optimizer to remove the call if the GPU is <GFX10.
When built at O0 it caused codegen issues as Clang allowed this intrinsic to go through with just +gfx10-insts, but the backend wanted the GPU to be >=GFX10 as well.

This patch allows selecting that intrinsic when just minimum required attributes are present. That is, +gfx10-insts & +16-bit-insts.

Depends on D136944

[foad]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

Yuck. Why is this required?

[Pierre-vh]

Fix getConstantBusLimit

[Pierre-vh]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

Ah, this is indeed ugly and I forgot to fix it.
Fixed it.

The reason why it's needed is because legalizeOperandsVOP3 sort of assumes (and rightly so) that it'll always return 2 for permlane(x).
If it returns 1, it messes up the legalization because the loop below the permlane-specific logic will undo the legalization.
Also if it returns 1 for PERMLANE, verification will fail complaining about constant bus limit being exceeded, but the instruction needs 2 sgpr operands so it doesn't make sense anyway.

[foad]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

What happens if you don't change this at all?

[foad]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

(Sorry I see you already answered that.)

[foad]

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

This still makes me nervous. Are there parts of the compiler that assume this'll return 1 for pre-GFX10 instructions? Have you tried compiling a large body of code with -mcpu=gfx900 -mattr=+gfx10insts to see if the compiler crashes horribly?

Maybe it would be cleaner to special-case v_permlane in legalizeOperandsVOP3 and/or verifyInstruction.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

557

Nothing here requires or implies a need for mad64_32?

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

I'd also rather not touch this.

This is another case where I feel we would be better off directly emitting subtarget specific instructions and reading the property out of the instruction properties

[Pierre-vh]

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

557

I don't really know, the assert was just tripping for some reason. Maybe I can just remove the assert and that's it?

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

185

What could be the alternative then? Just special-casing the instructions? Or not touching getConstantBusLimit at all?
I'm not sure the latter is a good idea, it should really return 2 for those instructions, no?

llvm/lib/Target/AMDGPU/AMDGPU.td

 def isGFX10Only :
   Predicate<"Subtarget->getGeneration() == AMDGPUSubtarget::GFX10">,
   AssemblerPredicate<(all_of FeatureGFX10Insts, (not FeatureGFX11Insts))>;
 
 def isGFX10Plus :
   Predicate<"Subtarget->getGeneration() >= AMDGPUSubtarget::GFX10">,
   AssemblerPredicate<(all_of FeatureGFX10Insts)>;
 
+def HasGFX10Insts :
+  Predicate<"Subtarget->hasGFX10Insts()">,
+  AssemblerPredicate<(all_of FeatureGFX10Insts)>;
+
 def isGFX10Before1030 :
   Predicate<"Subtarget->getGeneration() == AMDGPUSubtarget::GFX10 &&"
             "!Subtarget->hasGFX10_3Insts()">,
   AssemblerPredicate<(all_of FeatureGFX10Insts,(not FeatureGFX10_3Insts))>;
 
 def isGFX9GFX10 :
   Predicate<"Subtarget->getGeneration() == AMDGPUSubtarget::GFX9 ||"
             "Subtarget->getGeneration() == AMDGPUSubtarget::GFX10">,

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

if (ST.hasVOP3PInsts() && ST.hasAddNoCarry() && ST.hasIntClamp()) {
 
     getActionDefinitionsBuilder({G_UADDSAT, G_USUBSAT, G_SADDSAT, G_SSUBSAT})
       .legalFor({S32, S16, V2S16}) // Clamp modifier
       .minScalarOrElt(0, S16)
       .clampMaxNumElementsStrict(0, S16, 2)
       .scalarize(0)
       .widenScalarToNextPow2(0, 32)
       .lower();
-  } else if (ST.has16BitInsts()) {
+  } else if (ST.has16BitInsts() && ST.hasMad64_32()) {
     getActionDefinitionsBuilder({G_ADD, G_SUB})
       .legalFor({S32, S16})
       .minScalar(0, S16)
       .widenScalarToNextMultipleOf(0, 32)
       .maxScalar(0, S32)
       .scalarize(0);
 
     getActionDefinitionsBuilder(G_MUL)
         .legalFor({S32, S16})
         .scalarize(0)
         .minScalar(0, S16)
         .widenScalarToNextMultipleOf(0, 32)
         .custom();
-    assert(ST.hasMad64_32());
 
     // Technically the saturating operations require clamp bit support, but this
     // was introduced at the same time as 16-bit operations.
     getActionDefinitionsBuilder({G_UADDSAT, G_USUBSAT})
       .legalFor({S32, S16}) // Clamp modifier
       .minScalar(0, S16)
       .scalarize(0)
       .widenScalarToNextPow2(0, 16)

llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp

GCNSubtarget::GCNSubtarget(const Triple &TT, StringRef GPU, StringRef FS,
   InlineAsmLoweringInfo.reset(new InlineAsmLowering(getTargetLowering()));
   Legalizer.reset(new AMDGPULegalizerInfo(*this, TM));
   RegBankInfo.reset(new AMDGPURegisterBankInfo(*this));
   InstSelector.reset(new AMDGPUInstructionSelector(
   *this, *static_cast<AMDGPURegisterBankInfo *>(RegBankInfo.get()), TM));
 }
 
 unsigned GCNSubtarget::getConstantBusLimit(unsigned Opcode) const {
-  if (getGeneration() < GFX10)
+  if (!hasGFX10Insts())
     return 1;
 
   switch (Opcode) {
   case AMDGPU::V_LSHLREV_B64_e64:
   case AMDGPU::V_LSHLREV_B64_gfx10:
   case AMDGPU::V_LSHLREV_B64_e64_gfx11:
   case AMDGPU::V_LSHL_B64_e64:
   case AMDGPU::V_LSHRREV_B64_e64:

llvm/lib/Target/AMDGPU/GCNSubtarget.h

public:
 
   bool hasScalarAtomics() const {
     return HasScalarAtomics;
   }
 
   bool hasLDSFPAtomicAdd() const { return GFX8Insts; }
 
   /// \returns true if the subtarget has the v_permlanex16_b32 instruction.
-  bool hasPermLaneX16() const { return getGeneration() >= GFX10; }
+  bool hasPermLaneX16() const { return hasGFX10Insts(); }
 
   /// \returns true if the subtarget has the v_permlane64_b32 instruction.
   bool hasPermLane64() const { return getGeneration() >= GFX11; }
 
+  bool hasGFX10Insts() const { return GFX10Insts; }
+
   bool hasGFX11Insts() const { return GFX11Insts; }
 
   bool hasDPP() const {
     return HasDPP;
   }
 
   bool hasDPPBroadcasts() const {
     return HasDPP && getGeneration() < GFX10;

llvm/lib/Target/AMDGPU/SIInstrInfo.cpp

if (ST.hasGFX90AInsts()) {
     if (NMCOp == (uint16_t)-1)
       NMCOp = AMDGPU::getMCOpcode(Opcode, SIEncodingFamily::GFX90A);
     if (NMCOp == (uint16_t)-1)
       NMCOp = AMDGPU::getMCOpcode(Opcode, SIEncodingFamily::GFX9);
     if (NMCOp != (uint16_t)-1)
       MCOp = NMCOp;
   }
 
+  if (ST.hasGFX10Insts()) {
+    uint16_t NMCOp = AMDGPU::getMCOpcode(Opcode, SIEncodingFamily::GFX10);
+    if (NMCOp != (uint16_t)-1)
+      MCOp = NMCOp;
+  }
+
   if (ST.hasGFX11Insts()) {
     uint16_t NMCOp = AMDGPU::getMCOpcode(Opcode, SIEncodingFamily::GFX11);
     if (NMCOp != (uint16_t)-1)
       MCOp = NMCOp;
   }
 
   // (uint16_t)-1 means that Opcode is a pseudo instruction that has
   // no encoding in the given subtarget generation.

llvm/lib/Target/AMDGPU/VOP3Instructions.td

class PermlaneDiscardVDstIn<SDPatternOperator permlane,
   (permlane srcvalue, i32:$src0, i32:$src1, i32:$src2,
             timm:$fi, timm:$bc),
   (inst (as_i1timm $fi), VGPR_32:$src0, (as_i1timm $bc),
         SCSrc_b32:$src1, 0, SCSrc_b32:$src2,
         (IMPLICIT_DEF))
 >;
 
 
-let SubtargetPredicate = isGFX10Plus in {
+let Predicates = [HasGFX10Insts, Has16BitInsts] in {
   let isCommutable = 1, isReMaterializable = 1 in {
     defm V_XOR3_B32 : VOP3Inst <"v_xor3_b32", VOP3_Profile<VOP_I32_I32_I32_I32>>;
   } // End isCommutable = 1, isReMaterializable = 1
   def : ThreeOp_i32_Pats<xor, xor, V_XOR3_B32_e64>;
 
   let Constraints = "$vdst = $vdst_in", DisableEncoding="$vdst_in" in {
     defm V_PERMLANE16_B32 : VOP3Inst<"v_permlane16_b32", VOP3_PERMLANE_Profile>;
     defm V_PERMLANEX16_B32 : VOP3Inst<"v_permlanex16_b32", VOP3_PERMLANE_Profile>;

llvm/test/CodeGen/AMDGPU/llvm.amdgcn.permlane.ll

+; RUN: llc -amdgpu-load-store-vectorizer=0 -march=amdgcn -mattr=+gfx10-insts,+16-bit-insts -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX10PLUS,GFX10 %s
 ; RUN: llc -amdgpu-load-store-vectorizer=0 -march=amdgcn -mcpu=gfx1010 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX10PLUS,GFX10 %s
 ; RUN: llc -amdgpu-load-store-vectorizer=0 -march=amdgcn -mcpu=gfx1100 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX10PLUS,GFX11 %s
 
 declare i32 @llvm.amdgcn.permlane16(i32, i32, i32, i32, i1, i1) #1
 declare i32 @llvm.amdgcn.permlanex16(i32, i32, i32, i32, i1, i1) #1
 declare i32 @llvm.amdgcn.workitem.id.x()
 declare i32 @llvm.amdgcn.workitem.id.y()