[PATCH] D143293: AMDGPU: Use module flag to get code object version at IR level folow-up
Changpeng Fang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 6 10:31:40 PST 2023
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
More information about the llvm-commits
mailing list