cfang added a comment.
================ Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:5598 return ParseDirectiveHSAMetadata(); } else { - if (IDVal == ".hsa_code_object_version") ---------------- Are you sure Non-HSA does not have the four directives you deleted? ================ Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:122 std::optional<uint8_t> getHsaAbiVersion(const MCSubtargetInfo *STI) { if (STI && STI->getTargetTriple().getOS() != Triple::AMDHSA) return std::nullopt; ---------------- It is fine now. But I think STI could never be null. ================ Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h:46 enum { - AMDHSA_COV2 = 2, AMDHSA_COV3 = 3, ---------------- Should we keep this field, and just mention "unsupported"? ================ 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, ---------------- Are all these "isHsaAbiVersionX" no longer needed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146023/new/ https://reviews.llvm.org/D146023 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits