[AMDGPU] Allow frontends to disable null export for pixel shaders

[critson]

Summary

Disable null export (for kills) when a frontend defines a pixel
shader as not exporting using amdgpu-color-export and
amdgpu-depth-export function attrbutes.
This allows the generation of export free pixel shaders.

[foad]

LGTM.

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1353

Do these newly invented attributes need to be documented somewhere?

[critson]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1353

I cannot find anywhere the existing ones like InitialPSInputAddr are documented?

[foad]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1353

OK then I guess you're off the hook :-/

[ruiling]

llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp

76

Do you think we need to define a subtarget feature like AllowNoExportPS and check it here? The hardware behavior is different before and after gfx10, if we do not check this hardware difference, then this attributes will be gpu-generation-dependent (frontend should not set this attribute before gfx10). I am not sure whether this sounds a issue to others?

[critson]

llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp

76

It depends, it seems we are going in the direction that it is up to frontends to ensure correct behaviour of exports. Rather than adding a subtarget feature we could just add an architecture check here.

[ruiling]

llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp

76

Frontends are responsible for correctly inserting exp/exp_done to meet hardware requirement for most cases, there is only one exception AFAIK, the kill case which is what we are dealing with here. Architecture check sounds quite good to me. That would make the attribute just match the high level shader behavior.

[critson]

• Only allow no export PS on GFX10

[critson]

• Rename attributes to HasColorExport and HasDepthExport

[arsenm]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1355

Should stick to lowercase - separated naming convention with amdgpu- prefix

[critson]

• Rename function attributes to amdgpu-color-export and amdgpu-depth-export

[ruiling]

The logic looks good to me now. Any concern from others?

[foad]

LGTM.

llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp

75

Maybe add a brief comment explaining what changed in the hardware in GFX10?

[critson]

• Add comments

[critson]

@arsenm are you happy for this to proceed now?

[foad]

llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp

84

Typo "once" :)

[critson]

• Fix typo in comment

llvm/lib/Target/AMDGPU/SIDefines.h

enum Target : unsigned {
   ET_PRIM_MAX_IDX = 0,
   ET_MRT_MAX_IDX = 7,
   ET_POS_MAX_IDX = 4,
   ET_PARAM_MAX_IDX = 31,
 
   ET_INVALID = 255,
 };
 
+enum Format : unsigned {
+  SPI_SHADER_ZERO = 0x0,
+  SPI_SHADER_32_R = 0x1,
+  SPI_SHADER_32_GR = 0x2,
+  SPI_SHADER_32_AR = 0x3,
+  SPI_SHADER_FP16_ABGR = 0x4,
+  SPI_SHADER_UNORM16_ABGR = 0x5,
+  SPI_SHADER_SNORM16_ABGR = 0x6,
+  SPI_SHADER_UINT16_ABGR = 0x7,
+  SPI_SHADER_SINT16_ABGR = 0x8,
+  SPI_SHADER_32_ABGR = 0x9,
+};
+
 } // namespace Exp
 
 namespace VOP3PEncoding {
 
 enum OpSel : uint64_t {
   OP_SEL_HI_0 = UINT64_C(1) << 59,
   OP_SEL_HI_1 = UINT64_C(1) << 60,
   OP_SEL_HI_2 = UINT64_C(1) << 14,

llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp

 INITIALIZE_PASS_DEPENDENCY(MachineDominatorTree)
 INITIALIZE_PASS_END(SILateBranchLowering, DEBUG_TYPE,
                     "SI insert s_cbranch_execz instructions", false, false)
 
 char &llvm::SILateBranchLoweringPassID = SILateBranchLowering::ID;
 
 static void generateEndPgm(MachineBasicBlock &MBB,
                            MachineBasicBlock::iterator I, DebugLoc DL,
-                           const SIInstrInfo *TII, bool IsPS) {
+                           const SIInstrInfo *TII, MachineFunction &MF) {
+  const Function &F = MF.getFunction();
+  bool IsPS = F.getCallingConv() == CallingConv::AMDGPU_PS;
+  bool HasExports = AMDGPU::getPSColFormat(F) != AMDGPU::Exp::SPI_SHADER_ZERO ||
+                    AMDGPU::getPSZFormat(F) != AMDGPU::Exp::SPI_SHADER_ZERO;
   // "null export"
-  if (IsPS) {
+  if (IsPS && HasExports) {
     BuildMI(MBB, I, DL, TII->get(AMDGPU::EXP_DONE))
         .addImm(AMDGPU::Exp::ET_NULL)
         .addReg(AMDGPU::VGPR0, RegState::Undef)
         .addReg(AMDGPU::VGPR0, RegState::Undef)
         .addReg(AMDGPU::VGPR0, RegState::Undef)
         .addReg(AMDGPU::VGPR0, RegState::Undef)
         .addImm(1)  // vm
         .addImm(0)  // compr
         .addImm(0); // en
   }
   // s_endpgm
   BuildMI(MBB, I, DL, TII->get(AMDGPU::S_ENDPGM)).addImm(0);
 }
 
 static void splitBlock(MachineBasicBlock &MBB, MachineInstr &MI,
                        MachineDominatorTree *MDT) {
bool SILateBranchLowering::runOnMachineFunction(MachineFunction &MF) {
   if (!EarlyTermInstrs.empty()) {
     MachineBasicBlock *EarlyExitBlock = MF.CreateMachineBasicBlock();
     DebugLoc DL;
 
     MF.insert(MF.end(), EarlyExitBlock);
     BuildMI(*EarlyExitBlock, EarlyExitBlock->end(), DL, TII->get(MovOpc),
             ExecReg)
         .addImm(0);
-    generateEndPgm(*EarlyExitBlock, EarlyExitBlock->end(), DL, TII,
-                   MF.getFunction().getCallingConv() == CallingConv::AMDGPU_PS);
+    generateEndPgm(*EarlyExitBlock, EarlyExitBlock->end(), DL, TII, MF);
 
     for (MachineInstr *Instr : EarlyTermInstrs) {
       // Early termination in GS does nothing
       if (MF.getFunction().getCallingConv() != CallingConv::AMDGPU_GS)
         earlyTerm(*Instr, EarlyExitBlock);
       Instr->eraseFromParent();
     }
 

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h

uint64_t encodeMsg(uint64_t MsgId,
                    uint64_t OpId,
                    uint64_t StreamId);
 
 } // namespace SendMsg
 
 
 unsigned getInitialPSInputAddr(const Function &F);
 
+unsigned getPSColFormat(const Function &F);
+
+unsigned getPSZFormat(const Function &F);
+
 LLVM_READNONE
 bool isShader(CallingConv::ID CC);
 
 LLVM_READNONE
 bool isGraphics(CallingConv::ID CC);
 
 LLVM_READNONE
 bool isCompute(CallingConv::ID CC);

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

 //===----------------------------------------------------------------------===//
 //
 //===----------------------------------------------------------------------===//
 
 unsigned getInitialPSInputAddr(const Function &F) {
   return getIntegerAttribute(F, "InitialPSInputAddr", 0);
 }
 
+unsigned getPSColFormat(const Function &F) {
+  // As a safe default always respond as if there is one export.
+  return getIntegerAttribute(F, "PSColFormat", Exp::SPI_SHADER_32_ABGR);
+}
+
+unsigned getPSZFormat(const Function &F) {
+  return getIntegerAttribute(F, "PSZFormat", Exp::SPI_SHADER_ZERO);
+}
+
 bool isShader(CallingConv::ID cc) {
   switch(cc) {
     case CallingConv::AMDGPU_VS:
     case CallingConv::AMDGPU_LS:
     case CallingConv::AMDGPU_HS:
     case CallingConv::AMDGPU_ES:
     case CallingConv::AMDGPU_GS:
     case CallingConv::AMDGPU_PS:

llvm/test/CodeGen/AMDGPU/early-term.mir

--- |
 
   define amdgpu_gs void @early_term_scc0_gs() {
     ret void
   }
 
   define amdgpu_cs void @early_term_scc0_cs() {
     ret void
   }
+
+  define amdgpu_ps void @early_term_no_export() #0 {
+    ret void
+  }
+
+  attributes #0 = { "PSColFormat"="0" "PSZFormat"="0" }
 ...
 
 ---
 name: early_term_scc0_end_block
 tracksRegLiveness: true
 liveins:
   - { reg: '$sgpr0' }
   - { reg: '$sgpr1' }
body: |
   ; GFX10: bb.0:
   ; GFX10:   successors: %bb.1(0x80000000)
   ; GFX10:   liveins: $sgpr0, $sgpr1
   ; GFX10:   $vgpr0 = V_MOV_B32_e32 0, implicit $exec
   ; GFX10:   dead $sgpr0 = S_AND_B32 $sgpr0, killed $sgpr1, implicit-def $scc
   ; GFX10: bb.1:
   ; GFX10:   liveins: $vgpr0
   ; GFX10:   S_ENDPGM 0
+  ; GFX10: bb.2:
+  ; GFX10:   $exec_lo = S_MOV_B32 0
+  ; GFX10:   S_ENDPGM 0
   bb.0:
     liveins: $sgpr0, $sgpr1
     successors: %bb.1
 
     $vgpr0 = V_MOV_B32_e32 0, implicit $exec
     dead $sgpr0 = S_AND_B32 $sgpr0, killed $sgpr1, implicit-def $scc
     SI_EARLY_TERMINATE_SCC0 implicit $scc, implicit $exec
 
bb.0:
     $vgpr0 = V_MOV_B32_e32 0, implicit $exec
     dead $sgpr0 = S_AND_B32 $sgpr0, killed $sgpr1, implicit-def $scc
     SI_EARLY_TERMINATE_SCC0 implicit $scc, implicit $exec
 
   bb.1:
     liveins: $vgpr0
     S_ENDPGM 0
 ...
+
+---
+name: early_term_no_export
+tracksRegLiveness: true
+liveins:
+  - { reg: '$sgpr0' }
+  - { reg: '$sgpr1' }
+body: |
+  ; GFX10-LABEL: name: early_term_no_export
+  ; GFX10: bb.0:
+  ; GFX10:   successors: %bb.1(0x80000000), %bb.2(0x00000000)
+  ; GFX10:   liveins: $sgpr0, $sgpr1
+  ; GFX10:   $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+  ; GFX10:   dead $sgpr0 = S_AND_B32 $sgpr0, killed $sgpr1, implicit-def $scc
+  ; GFX10:   S_CBRANCH_SCC0 %bb.2, implicit $scc
+  ; GFX10: bb.1:
+  ; GFX10:   liveins: $vgpr0
+  ; GFX10:   EXP_DONE 0, $vgpr0, $vgpr0, $vgpr0, $vgpr0, -1, -1, 15, implicit $exec
+  ; GFX10:   S_ENDPGM 0
+  ; GFX10: bb.2:
+  ; GFX10:   $exec_lo = S_MOV_B32 0
+  ; GFX10:   S_ENDPGM 0
+  bb.0:
+    liveins: $sgpr0, $sgpr1
+    successors: %bb.1
+
+    $vgpr0 = V_MOV_B32_e32 0, implicit $exec
+    dead $sgpr0 = S_AND_B32 $sgpr0, killed $sgpr1, implicit-def $scc
+    SI_EARLY_TERMINATE_SCC0 implicit $scc, implicit $exec
+
+  bb.1:
+    liveins: $vgpr0
+    EXP_DONE 0, $vgpr0, $vgpr0, $vgpr0, $vgpr0, -1, -1, 15, implicit $exec
+    S_ENDPGM 0
+...