[PATCH] D146023: [AMDGPU] Remove Code Object V2

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5598
       return ParseDirectiveHSAMetadata();
   } else {
-    if (IDVal == ".hsa_code_object_version")
----------------
cfang wrote:
> Are you sure Non-HSA does not have the four directives you deleted?  
I'm really not sure, I saw `hsa` in the name and I thought it only applied to HSA, but some tests are failing.
I'll leave them in until someone can answer for sure.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h:46
 enum {
-  AMDHSA_COV2 = 2,
   AMDHSA_COV3 = 3,
----------------
cfang wrote:
> Should we keep this field, and just mention "unsupported"?
I'm not sure about that, I assume that we're removing as many traces of V2 as possible from the backend. No point in keeping an unused enum entry IMO, but I'm okay with keeping it if there's a good reason to


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h:59
 /// false otherwise.
 bool isHsaAbiVersion3(const MCSubtargetInfo *STI);
 /// \returns True if HSA OS ABI Version identification is 4,
----------------
cfang wrote:
> Are all these "isHsaAbiVersionX" no longer needed? 
Yes they're needed because they implicitly check for HSA as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146023