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

[Changpeng Fang via Phabricator via llvm-commits]

cfang marked 3 inline comments as done.
cfang added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:351
+    default:
+      llvm_unreachable("Unexpected code object version");
+      break;
----------------
arsenm wrote:
> 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
Plan to work on with the unreachable code object version as another patch. 
Add a test case as suggested.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:414
     bool SupportsGetDoorbellID = InfoCache.supportsGetDoorbellID(*F);
+    unsigned COV = AMDGPU::getCodeObjectVersion(*F->getParent());
 
----------------
arsenm wrote:
> cfang wrote:
> > arsenm wrote:
> > > Cann move this into AMDGPUInformationCache
> > Should we introduce a private field of Module *M in AMDGPUInformationCache? 
> > InformationCache itself does not have this field.
> > Otherwise, we will have to pass a Module parameter in the call to check code object version each time.
> > 
> You can directly parse the code object version into the info cache, it won't be shared between modules 
Keep code object version in AMDGPUInformationCache when the object is created.


================
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"] =
----------------
arsenm wrote:
> Only need to call this once (Can we cache it in the streamer or read from the an argument?)
Pass as an argument.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:172
   default:
     llvm_unreachable("Unexpected code object version");
     return 0;
----------------
arsenm wrote:
> As a separate patch need to stop unreachable on unhandled version
Plan to handle out of range code object version in another patch.


================
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:
----------------
arsenm wrote:
> Should also clean up all these argument offset queries to return a single struct or something
Have to think over what to do. Better to be in another patch.


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

https://reviews.llvm.org/D143138