[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;
----------------
cfang wrote:
> 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.
it's still a dead break, better to just not make this unreachable in the first place


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:141
+      : InformationCache(M, AG, Allocator, CGSCC), TM(TM) {
+    CodeObjectVersion = AMDGPU::getCodeObjectVersion(M);
+  }
----------------
can do it in initializer list


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:172
+  /// Get code object version.
+  unsigned getCodeObjectVersion() {
+    return CodeObjectVersion;
----------------
const 


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:233
+  
+  unsigned CodeObjectVersion;
 };
----------------
const


================
Comment at: llvm/test/CodeGen/AMDGPU/unsupported-code-object-version.ll:6
+; HSA-ERROR: Unexpected code object version
+; HSA-ERROR: UNREACHABLE executed at
+
----------------
you can't rely on this message, at least use report_fatal_error


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

https://reviews.llvm.org/D143138