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

[Jacob Lambert via Phabricator via llvm-commits]

lamb-j 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 \
----------------
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


================
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
 
----------------
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


================
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));
----------------
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


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

https://reviews.llvm.org/D129818