[PATCH] D143293: AMDGPU: Use module flag to get code object version at IR level folow-up

[Changpeng Fang via Phabricator via llvm-commits]

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

In D143293#4106251 <https://reviews.llvm.org/D143293#4106251>, @kosarev wrote:

> Do we still need `getHsaAbiVersion()` and the `ELFABIVERSION_AMDGPU_HSA_*` constants?

In term of code object version itself, I don't think these are still needed. However, ABIVersion is used in the assembler/disassembler,
and the work to check code object version (not from command line) is on-going, so it is better to get rid of them completely in that patch.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp:335
   CodeObjectVersion = AMDGPU::getCodeObjectVersion(M);
-
   if (TM.getTargetTriple().getOS() == Triple::AMDHSA) {
----------------
kosarev wrote:
> Is this an accidental change?
Right. Thanks for pointing out.


================
Comment at: llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUTargetStreamer.h:114-115
+  void initializeTargetID(const MCSubtargetInfo &STI, StringRef FeatureString,
+                          unsigned COV) {
+    initializeTargetID(STI, COV);
 
----------------
kosarev wrote:
> `COV` looks a bit cryptic here. What if `CodeObjectVersion`?
OK.  


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h:123
   TargetIDSetting SramEccSetting;
+  unsigned CodeObjectVersion;
 
----------------
kosarev wrote:
> We probably want this initialised -- regardless of whether it is then assigned a value?
Initialize it to 0, and I think this value will never been used.


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

https://reviews.llvm.org/D143293