[Clang][OpenMP] Fix handling of -mcode-object-version for OpenMP
Needs ReviewPublic
Actions

Authored by saiislam on Jan 18 2023, 7:29 AM.

Details

Reviewers

jhuber6
yaxunl
jdoerfert

Summary

Code object version flag for AMDGPU was not being honored by the
driver. It was required to be passed as derived arg so that correct
bitcode library can be linked.

Diff Detail

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	3,610 ms	libcxx CI Modules > llvm-libc++-shared-cfg-in.libcxx/algorithms/specialized_algorithms/special_mem_concepts::nothrow_sentinel_for.compile.pass.cpp Script: -- : 'COMPILED WITH'; /home/libcxx-builder/.buildkite-agent/builds/859ecda2a84a-1/llvm-project/libcxx-ci/install/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/859ecda2a84a-1/llvm-project/libcxx-ci/libcxx/test/libcxx/algorithms/specialized.algorithms/special.mem.concepts/nothrow_sentinel_for.compile.pass.cpp --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/859ecda2a84a-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/859ecda2a84a-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/859ecda2a84a-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -fsyntax-only
	1,440 ms	x64 debian > Clang.Driver::amdgpu-openmp-toolchain.c Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgpu-bc-path=/var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/Inputs/hip_dev_lib /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/amdgpu-openmp-toolchain.c 2>&1 \| /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/amdgpu-openmp-toolchain.c

Event Timeline

saiislam created this revision.Jan 18 2023, 7:29 AM

Herald added a project: Restricted Project. · View Herald TranscriptJan 18 2023, 7:29 AM

Herald added subscribers: kosarev, kerbowa, guansong and 2 others. · View Herald Transcript

saiislam requested review of this revision.Jan 18 2023, 7:29 AM

Herald added a reviewer: jdoerfert. · View Herald TranscriptJan 18 2023, 7:29 AM

Herald added a project: Restricted Project. · View Herald Transcript

Herald added subscribers: cfe-commits, sstefan1, MaskRay. · View Herald Transcript

saiislam mentioned this in D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5.Jan 18 2023, 7:31 AM

saiislam added a parent revision: D140784: [OpenMP][AMDGPU] Introduce new matadata fields for code object 5.

Harbormaster completed remote builds in B208489: Diff 490152.Jan 18 2023, 8:07 AM

yaxunl added inline comments.Jan 18 2023, 8:21 AM

clang/lib/Driver/ToolChains/Clang.cpp
7116	why do you need to change Args to C.getArgs() ?

Removed the unnecessary call to getArgs() and added test cases.

saiislam marked an inline comment as done.Jan 18 2023, 9:39 AM

saiislam added inline comments.

clang/lib/Driver/ToolChains/Clang.cpp
7116	You are right. Don't need it. Removed.

Harbormaster completed remote builds in B208531: Diff 490204.Jan 18 2023, 11:26 AM

Is this patch really necessary? -mcode-object-version= option does not go through any translation in derived args. If it is in derived args, it should also be in the original args. HIPAMD toolchain just uses the original driver args. Why OpenMP toolchain needs to use the derived args?