[PATCH] D63639: [AMDGPU] Prevent backend override of WGP when using PAL

[David Stuttard via Phabricator via llvm-commits]

dstuttard marked an inline comment as done.
dstuttard added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:970-974
+    // Leave WgpMode as default (0) for PAL - PAL will set this bit and should
+    // not be overridden here
+    ProgInfo.WgpMode = (STM.isCuModeEnabled() ||
+                        TM.getTargetTriple().getOS() == Triple::AMDPAL)
+                           ? 0 : 1;
----------------
arsenm wrote:
> arsenm wrote:
> > This seems like a workaround. Why isn't PAL setting what it wants initially and overriding it later?
> I'm also thinking this should be an attribute rather than a sub target feature
Not sure what you're getting at here.
Currently PAL does set the value in the ComputePGMRSrc1 register. Policy is to OR any changes into the control registers that are input - but in this case that only works if you happen to enable CuMode (the default for the backend is to not be in CuMode, so WGP is always set to 1).

We could work around this by always setting the CuMode sub target feature in any PAL front-end - then the value passed in from PAL would always be honoured, but that seems more hacky to me.

We can change CuMode to be an attribute rather than sub target feature as a separate change, but that pre-exists this work.

I'll put a test together if we can agree an approach.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63639