[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
jhuber6 updated this revision to Diff 505166. jhuber6 added a comment. Adding release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 Files: clang/docs/ReleaseNotes.rst clang/lib/Driver/ToolChains/AMDGPU.cpp clang/test/Driver/amdgpu-toolchain-opencl.cl clang/test/Driver/amdgpu-toolchain.c Index: clang/test/Driver/amdgpu-toolchain.c === --- clang/test/Driver/amdgpu-toolchain.c +++ clang/test/Driver/amdgpu-toolchain.c @@ -6,7 +6,7 @@ // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" -// AS_LINK: ld.lld{{.*}} "-shared" +// AS_LINK: ld.lld{{.*}} "--no-undefined" "-shared" // DWARF_VER: "-dwarf-version=5" Index: clang/test/Driver/amdgpu-toolchain-opencl.cl === --- clang/test/Driver/amdgpu-toolchain-opencl.cl +++ clang/test/Driver/amdgpu-toolchain-opencl.cl @@ -25,3 +25,6 @@ // CHK-INC: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cl" // CHK-INC-NOT: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cpp-output" + +// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -emit-llvm -mcpu=fiji %s 2>&1 | FileCheck -check-prefix=CHK-LINK %s +// CHK-LINK: ld.lld{{.*}} "--no-undefined" "-shared" Index: clang/lib/Driver/ToolChains/AMDGPU.cpp === --- clang/lib/Driver/ToolChains/AMDGPU.cpp +++ clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -543,6 +543,7 @@ if (C.getDriver().isUsingLTO()) addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], C.getDriver().getLTOMode() == LTOK_Thin); + CmdArgs.push_back("--no-undefined"); CmdArgs.push_back("-shared"); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); Index: clang/docs/ReleaseNotes.rst === --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -222,6 +222,14 @@ Target Specific Changes --- +AMDGPU Support +^^ + +- Linking for AMDGPU now uses ``--no-undefined`` by default. This causes + undefined symbols in the created module to be a linker error. To prevent this, + pass ``-Wl,--undefined`` if compiling directly, or ``-Xoffload-linker + --undefined`` if using an offloading language. + X86 Support ^^^ Index: clang/test/Driver/amdgpu-toolchain.c === --- clang/test/Driver/amdgpu-toolchain.c +++ clang/test/Driver/amdgpu-toolchain.c @@ -6,7 +6,7 @@ // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" -// AS_LINK: ld.lld{{.*}} "-shared" +// AS_LINK: ld.lld{{.*}} "--no-undefined" "-shared" // DWARF_VER: "-dwarf-version=5" Index: clang/test/Driver/amdgpu-toolchain-opencl.cl === --- clang/test/Driver/amdgpu-toolchain-opencl.cl +++ clang/test/Driver/amdgpu-toolchain-opencl.cl @@ -25,3 +25,6 @@ // CHK-INC: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cl" // CHK-INC-NOT: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cpp-output" + +// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -emit-llvm -mcpu=fiji %s 2>&1 | FileCheck -check-prefix=CHK-LINK %s +// CHK-LINK: ld.lld{{.*}} "--no-undefined" "-shared" Index: clang/lib/Driver/ToolChains/AMDGPU.cpp === --- clang/lib/Driver/ToolChains/AMDGPU.cpp +++ clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -543,6 +543,7 @@ if (C.getDriver().isUsingLTO()) addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], C.getDriver().getLTOMode() == LTOK_Thin); + CmdArgs.push_back("--no-undefined"); CmdArgs.push_back("-shared"); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); Index: clang/docs/ReleaseNotes.rst === --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -222,6 +222,14 @@ Target Specific Changes --- +AMDGPU Support +^^ + +- Linking for AMDGPU now uses ``--no-undefined`` by default. This causes + undefined symbols in the created module to be a linker error. To prevent this, + pass ``-Wl,--undefined`` if compiling directly, or ``-Xoffload-linker + --undefined`` if using an offloading language. + X86 Support ^^^ ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
arsenm added a comment. Release note change should be here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. While `--no-undefined` is an alias for `-z defs`, be consistent in the subject and the body. > Always use -zdefs `--no-undefined` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
jhuber6 updated this revision to Diff 504818. jhuber6 added a comment. Use `--no-undefined` to be consistent with HIP and check for OpenCL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 Files: clang/lib/Driver/ToolChains/AMDGPU.cpp clang/test/Driver/amdgpu-toolchain-opencl.cl clang/test/Driver/amdgpu-toolchain.c Index: clang/test/Driver/amdgpu-toolchain.c === --- clang/test/Driver/amdgpu-toolchain.c +++ clang/test/Driver/amdgpu-toolchain.c @@ -6,7 +6,7 @@ // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" -// AS_LINK: ld.lld{{.*}} "-shared" +// AS_LINK: ld.lld{{.*}} "--no-undefined" "-shared" // DWARF_VER: "-dwarf-version=5" Index: clang/test/Driver/amdgpu-toolchain-opencl.cl === --- clang/test/Driver/amdgpu-toolchain-opencl.cl +++ clang/test/Driver/amdgpu-toolchain-opencl.cl @@ -25,3 +25,6 @@ // CHK-INC: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cl" // CHK-INC-NOT: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cpp-output" + +// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -emit-llvm -mcpu=fiji %s 2>&1 | FileCheck -check-prefix=CHK-LINK %s +// CHK-LINK: ld.lld{{.*}} "--no-undefined" "-shared" Index: clang/lib/Driver/ToolChains/AMDGPU.cpp === --- clang/lib/Driver/ToolChains/AMDGPU.cpp +++ clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -543,6 +543,7 @@ if (C.getDriver().isUsingLTO()) addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], C.getDriver().getLTOMode() == LTOK_Thin); + CmdArgs.push_back("--no-undefined"); CmdArgs.push_back("-shared"); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); Index: clang/test/Driver/amdgpu-toolchain.c === --- clang/test/Driver/amdgpu-toolchain.c +++ clang/test/Driver/amdgpu-toolchain.c @@ -6,7 +6,7 @@ // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" -// AS_LINK: ld.lld{{.*}} "-shared" +// AS_LINK: ld.lld{{.*}} "--no-undefined" "-shared" // DWARF_VER: "-dwarf-version=5" Index: clang/test/Driver/amdgpu-toolchain-opencl.cl === --- clang/test/Driver/amdgpu-toolchain-opencl.cl +++ clang/test/Driver/amdgpu-toolchain-opencl.cl @@ -25,3 +25,6 @@ // CHK-INC: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cl" // CHK-INC-NOT: "-cc1" {{.*}}"-finclude-default-header" "-fdeclare-opencl-builtins" {{.*}}"-x" "cpp-output" + +// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -emit-llvm -mcpu=fiji %s 2>&1 | FileCheck -check-prefix=CHK-LINK %s +// CHK-LINK: ld.lld{{.*}} "--no-undefined" "-shared" Index: clang/lib/Driver/ToolChains/AMDGPU.cpp === --- clang/lib/Driver/ToolChains/AMDGPU.cpp +++ clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -543,6 +543,7 @@ if (C.getDriver().isUsingLTO()) addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], C.getDriver().getLTOMode() == LTOK_Thin); + CmdArgs.push_back("--no-undefined"); CmdArgs.push_back("-shared"); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
jhuber6 added a comment. This can be turned off with `-zundefs`. So we could instruct people to use `-Wl,-zundefs` or `-Xoffload-linker -zundefs` if the old behavior is desired. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
JonChesterfield added a reviewer: AMDGPU. JonChesterfield added a comment. Adding the amdgpu reviewer group. This change might be contentious - we currently treat various broken code as calls to trap on the grounds that those functions might not be executed. By analogy, functions that call undefined functions but are not themselves called might be considered not a problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145941/new/ https://reviews.llvm.org/D145941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D145941: [Clang] Always use -zdefs when linking AMDGPU images
jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, arsenm, yaxunl, MaskRay. Herald added subscribers: kosarev, kerbowa, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this revision. Herald added subscribers: cfe-commits, wdng. Herald added a project: clang. AMDGPU uses ELF shared libraries to implement their executable device images. One downside to this method is that it disables regular warnings on undefined symbols. This is because shared libraries expect these to be resolves by later loads. However, the GPU images do not support dynamic linking so any undefined symbol is going to cause a runtime error. This patch adds `-zdefs` to the `ld.lld` invocation to guaruntee that undefined symbols are always caught as linking errors rather than runtime errors. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145941 Files: clang/lib/Driver/ToolChains/AMDGPU.cpp clang/test/Driver/amdgpu-toolchain.c Index: clang/test/Driver/amdgpu-toolchain.c === --- clang/test/Driver/amdgpu-toolchain.c +++ clang/test/Driver/amdgpu-toolchain.c @@ -6,7 +6,7 @@ // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" -// AS_LINK: ld.lld{{.*}} "-shared" +// AS_LINK: ld.lld{{.*}} "-shared" "-zdefs" // DWARF_VER: "-dwarf-version=5" Index: clang/lib/Driver/ToolChains/AMDGPU.cpp === --- clang/lib/Driver/ToolChains/AMDGPU.cpp +++ clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -544,6 +544,7 @@ addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], C.getDriver().getLTOMode() == LTOK_Thin); CmdArgs.push_back("-shared"); + CmdArgs.push_back("-zdefs"); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); C.addCommand(std::make_unique( Index: clang/test/Driver/amdgpu-toolchain.c === --- clang/test/Driver/amdgpu-toolchain.c +++ clang/test/Driver/amdgpu-toolchain.c @@ -6,7 +6,7 @@ // RUN: %clang -### -g --target=amdgcn-mesa-mesa3d -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s // AS_LINK: "-cc1as" -// AS_LINK: ld.lld{{.*}} "-shared" +// AS_LINK: ld.lld{{.*}} "-shared" "-zdefs" // DWARF_VER: "-dwarf-version=5" Index: clang/lib/Driver/ToolChains/AMDGPU.cpp === --- clang/lib/Driver/ToolChains/AMDGPU.cpp +++ clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -544,6 +544,7 @@ addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0], C.getDriver().getLTOMode() == LTOK_Thin); CmdArgs.push_back("-shared"); + CmdArgs.push_back("-zdefs"); CmdArgs.push_back("-o"); CmdArgs.push_back(Output.getFilename()); C.addCommand(std::make_unique( ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits