[AMDGPU] Prevent backend override of WGP when using PAL

[dstuttard]

Summary

Backend defaults to setting WGP mode to 0 or 1 depending on the cumode
feature. For PAL clients we want PAL to set this mode.
Adding check to prevent the backend overriding whatever the front end has set.

[arsenm]

Needs test

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

970–974

This seems like a workaround. Why isn't PAL setting what it wants initially and overriding it later?

[arsenm]

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

970–974

I'm also thinking this should be an attribute rather than a sub target feature

[dstuttard]

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

970–974

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.

[dstuttard]

ping

[nhaehnle]

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

970–974

Always setting the subtarget feature does seem like the right approach to me.

Changing it to a target attribute may be something to consider. It's one of those non-trivial things because it would have to be added to all functions, not just the kernel entry point, but...

[arsenm]

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

970–974

This seems like something we should handle consistently for all OS types, not just AMDPAL

[dstuttard]

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

970–974

I think I'll go with always setting the sub target feature for now, so I'll abandon this change.

[dstuttard]

I might revisit this one - setting cumode seems messy to enable driver control of the WGP setting, but seems the most pragmatic at the moment.

lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

void AMDGPUAsmPrinter::getSIProgramInfo(SIProgramInfo &ProgInfo,
   // is used by the entire wave.  ProgInfo.ScratchSize is the amount of
   // scratch memory used per thread.
   ProgInfo.ScratchBlocks =
       alignTo(ProgInfo.ScratchSize * STM.getWavefrontSize(),
               1ULL << ScratchAlignShift) >>
       ScratchAlignShift;
 
   if (getIsaVersion(getGlobalSTI()->getCPU()).Major >= 10) {
-    ProgInfo.WgpMode = STM.isCuModeEnabled() ? 0 : 1;
+    // 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;
     ProgInfo.MemOrdered = 1;
   }
 
   ProgInfo.ComputePGMRSrc1 =
       S_00B848_VGPRS(ProgInfo.VGPRBlocks) |
       S_00B848_SGPRS(ProgInfo.SGPRBlocks) |
       S_00B848_PRIORITY(ProgInfo.Priority) |
       S_00B848_FLOAT_MODE(ProgInfo.FloatMode) |