[PATCH] D129818: AMDGPU: Make default AMDHSA Code Object Version to be 5

[Changpeng Fang via Phabricator via llvm-commits]

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


================
Comment at: clang/test/Driver/hip-device-libs.hip:144
 
 // Test default code object version with old device library without abi_version_400.bc
 // RUN: %clang -### -target x86_64-linux-gnu --offload-arch=gfx900 \
----------------
lamb-j wrote:
> 500?
> 
> I think we may also need to update the comment here: https://github.com/llvm/llvm-project/blob/e9569748def85a2f6bb8066e516a84e6933ef407/clang/test/Driver/hip-device-libs.hip#LL196C1-L196C83
Yes. Should comment as 500 in these places. Thanks.


================
Comment at: clang/test/Driver/hip-device-libs.hip:154
 // RUN:   --rocm-path=%S/Inputs/rocm %S/Inputs/hip_multiple_inputs/b.hip \
 // RUN: 2>&1 | FileCheck %s --check-prefixes=ABI4
 
----------------
lamb-j wrote:
> Should this be ABI5? That is, if CodeObjectVersion < 4, should it be set to 5 (instead of 4)?
> 
> May also be related to a needed change here: https://github.com/llvm/llvm-project/blob/6db007a0654ed7a6ed5c3aa3b61a937c19a6bc6b/clang/lib/Driver/ToolChains/ROCm.h#L29
No. it should be 4 if <=4. I think this is mainly for device-lib versions (400.bc vs 500.bc). 


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:31-34
 static llvm::cl::opt<unsigned>
     AmdhsaCodeObjectVersion("amdhsa-code-object-version", llvm::cl::Hidden,
                             llvm::cl::desc("AMDHSA Code Object Version"),
+                            llvm::cl::init(5));
----------------
lamb-j wrote:
> We may also want to update this file here:
> 
> https://github.com/llvm/llvm-project/blob/e9569748def85a2f6bb8066e516a84e6933ef407/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp#L162
You are right. Done.


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

https://reviews.llvm.org/D129818