[PATCH] D143138: AMDGPU: Use module flag to get code object version at IR level

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:351
+    default:
+      llvm_unreachable("Unexpected code object version");
+      break;
----------------
Should not unreachable on unknown version. Either should error or just try to handle as the latest version. We have a few other unreachable that need fixing too.

Should add a test with 0/1 and a large number


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:414
     bool SupportsGetDoorbellID = InfoCache.supportsGetDoorbellID(*F);
+    unsigned COV = AMDGPU::getCodeObjectVersion(*F->getParent());
 
----------------
Cann move this into AMDGPUInformationCache


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp:897
         Kern.getDocument()->getNode(ProgramInfo.DynamicCallStack);
-  if (AMDGPU::getAmdhsaCodeObjectVersion() >= 5 && STM.supportsWGP())
+  if (AMDGPU::getCodeObjectVersion(*M) >= 5 && STM.supportsWGP())
     Kern[".workgroup_processor_mode"] =
----------------
Only need to call this once (Can we cache it in the streamer or read from the an argument?)


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUSubtarget.cpp:547
+  const Module *M = F.getParent();
+  unsigned NBytes = (AMDGPU::getCodeObjectVersion(*M) >= 5) ? 256 : 56;
   return F.getFnAttributeAsParsedInteger("amdgpu-implicitarg-num-bytes",
----------------
Don't need parens 


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:172
   default:
     llvm_unreachable("Unexpected code object version");
     return 0;
----------------
As a separate patch need to stop unreachable on unhandled version


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:194-203
+unsigned getDefaultQueueImplicitArgPosition(unsigned COV) {
   switch (AmdhsaCodeObjectVersion) {
   case 2:
   case 3:
   case 4:
     return 32;
   case 5:
----------------
Should also clean up all these argument offset queries to return a single struct or something


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

https://reviews.llvm.org/D143138