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

[Ruiling, Song via Phabricator via llvm-commits]

ruiling added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SILateBranchLowering.cpp:76
   // "null export"
-  if (IsPS) {
+  if (IsPS && HasExports) {
     BuildMI(MBB, I, DL, TII->get(AMDGPU::EXP_DONE))
----------------
critson wrote:
> ruiling wrote:
> > 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?
> 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105683