kzhuravl added inline comments.
================ Comment at: llvm/include/llvm/BinaryFormat/ELF.h:737 EF_AMDGPU_MACH_AMDGCN_FIRST = EF_AMDGPU_MACH_AMDGCN_GFX600, - EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX805, + EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX90A, ---------------- tra wrote: > Nit: This looks odd. GFX90A does not need to be in the middle of the list. It > makes it somewhat confusing to tell which ID is really the last. The `_LAST` > enum says it's GFX90A, but it's not the last item of the list. There are > already out-of-name-order GPUs at the end of the list, so putting 90A at the > end would probably be a better choice. At least we'd still have the numeric > values in order. Right now the list is ordered neither by the name nor by the > value. > > There's also a question of whether something needs to be done about the > missing values 0x3c..0x3e. Presumably the `_FIRST`..`_LAST` enums specify the > range we'll use to iterate over the GPU IDs. Do we handle the missing values > correctly? > > Looks like it's benign at the moment as we're only using it to return amdgcn > triple in ELFObjectFile.h. I'd add the placeholder enums for the > reserved/unused values within the range. > https://reviews.llvm.org/D97010 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96906/new/ https://reviews.llvm.org/D96906 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
