https://github.com/yxsamliu updated https://github.com/llvm/llvm-project/pull/228096
>From f4d578919ca8e4a0c9eebfb27f65ad637e4ffda1 Mon Sep 17 00:00:00 2001 From: "Yaxun (Sam) Liu" <[email protected]> Date: Thu, 1 Oct 2026 10:17:45 -0400 Subject: [PATCH 1/2] [clang][HIP] Apply clang-cl optimization options to device code clang-cl options such as /O1, /O2, and /Ox currently optimize only host code in HIP builds, leaving device code unoptimized. Translate these options before separating host and device arguments so both compilations receive the requested optimization settings. --- clang/docs/ReleaseNotes.md | 3 + clang/include/clang/Driver/Driver.h | 3 +- clang/lib/Driver/Driver.cpp | 130 +++++++++++++++++++++- clang/lib/Driver/ToolChains/MSVC.cpp | 130 +--------------------- clang/test/Driver/cl-options.cu | 7 ++ clang/test/Driver/hip-cl-optimization.hip | 106 ++++++++++++++++++ 6 files changed, 247 insertions(+), 132 deletions(-) create mode 100644 clang/test/Driver/hip-cl-optimization.hip diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index d22874d4130eec..4fdefafccea14d 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -815,6 +815,9 @@ features cannot lower the translation-unit ABI level; #### Miscellaneous Bug Fixes +- Fixed `clang-cl` optimization options such as `/O1`, `/O2`, and + `/Ox` being ignored when compiling HIP device code. + #### Miscellaneous Clang Crashes Fixed - Fixed a crash in CTAD for type alias templates when the aggregate deduction guide could not be resolved. (#GH206994) diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 15b6fdcb8a5746..914ef62cd62fa9 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -360,7 +360,8 @@ class Driver { /// TranslateInputArgs - Create a new derived argument list from the input /// arguments, after applying the standard argument translations. llvm::opt::DerivedArgList * - TranslateInputArgs(const llvm::opt::InputArgList &Args) const; + TranslateInputArgs(const llvm::opt::InputArgList &Args, + const llvm::Triple &Triple) const; // handleArguments - All code related to claiming and printing diagnostics // related to arguments to the driver are done here. diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 7649941a68b1cc..07f70fbbe0128c 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -459,10 +459,128 @@ Arg *clang::driver::makeInputArg(DerivedArgList &Args, const OptTable &Opts, return A; } -DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args) const { +static void translateMSVCOptArg(Arg *A, llvm::opt::DerivedArgList &DAL, + bool SupportsForcingFramePointer, + const char *ExpandChar, const OptTable &Opts) { + assert(A->getOption().matches(options::OPT__SLASH_O)); + + StringRef OptStr = A->getValue(); + for (size_t I = 0, E = OptStr.size(); I != E; ++I) { + const char &OptChar = *(OptStr.data() + I); + switch (OptChar) { + default: + break; + case '1': + case '2': + case 'x': + case 'd': + // Ignore /O[12xd] flags that aren't the last one on the command line. + // Only the last one gets expanded. + if (&OptChar != ExpandChar) { + A->claim(); + break; + } + if (OptChar == 'd') { + DAL.AddFlagArg(A, Opts.getOption(options::OPT_O0)); + } else { + if (OptChar == '1') { + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); + } else if (OptChar == '2' || OptChar == 'x') { + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); + } + if (SupportsForcingFramePointer && + !DAL.hasArgNoClaim(options::OPT_fno_omit_frame_pointer)) + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); + if (OptChar == '1' || OptChar == '2') + DAL.AddFlagArg(A, Opts.getOption(options::OPT_ffunction_sections)); + } + break; + case 'b': + if (I + 1 != E && isdigit(OptStr[I + 1])) { + switch (OptStr[I + 1]) { + case '0': + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_inline)); + break; + case '1': + DAL.AddFlagArg(A, + Opts.getOption(options::OPT_finline_hint_functions)); + break; + case '2': + case '3': + DAL.AddFlagArg(A, Opts.getOption(options::OPT_finline_functions)); + break; + } + ++I; + } + break; + case 'g': + A->claim(); + break; + case 'i': + if (I + 1 != E && OptStr[I + 1] == '-') { + ++I; + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_builtin)); + } else { + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); + } + break; + case 's': + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); + break; + case 't': + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); + break; + case 'y': { + bool OmitFramePointer = true; + if (I + 1 != E && OptStr[I + 1] == '-') { + OmitFramePointer = false; + ++I; + } + if (SupportsForcingFramePointer) { + if (OmitFramePointer) + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); + else + DAL.AddFlagArg(A, + Opts.getOption(options::OPT_fno_omit_frame_pointer)); + } else { + // Silently accept /Oy- on x86-64 for portable clang-cl build flags. + A->claim(); + } + break; + } + } + } +} + +DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args, + const llvm::Triple &Triple) const { const llvm::opt::OptTable &Opts = getOpts(); DerivedArgList *DAL = new DerivedArgList(Args); + // Normalize MSVC optimization options before host and device arguments split. + bool TranslateMSVCOpts = Triple.isWindowsMSVCEnvironment(); + // /Oy and /Oy- do not affect the x86-64 host. + bool SupportsForcingFramePointer = Triple.getArch() != llvm::Triple::x86_64; + // Expand only the last /O[12xd], preserving overrides such as /O2 /Oy-. + const char *ExpandChar = nullptr; + if (TranslateMSVCOpts) { + for (Arg *A : Args.filtered(options::OPT__SLASH_O)) { + StringRef OptStr = A->getValue(); + for (size_t I = 0, E = OptStr.size(); I != E; ++I) { + char OptChar = OptStr[I]; + char PrevChar = I > 0 ? OptStr[I - 1] : '0'; + if (PrevChar == 'b') { + // OptChar does not expand; it's an argument to the previous char. + continue; + } + if (OptChar == '1' || OptChar == '2' || OptChar == 'x' || + OptChar == 'd') + ExpandChar = OptStr.data() + I; + } + } + } + bool HasNostdlib = Args.hasArg(options::OPT_nostdlib); bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx); bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs); @@ -480,6 +598,14 @@ DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args) const { continue; } + if (TranslateMSVCOpts && A->getOption().matches(options::OPT__SLASH_O)) { + // Keep the original argument for unused-option diagnostics. + DAL->append(A); + translateMSVCOptArg(A, *DAL, SupportsForcingFramePointer, ExpandChar, + Opts); + continue; + } + // Unfortunately, we have to parse some forwarding options (-Xassembler, // -Xlinker, -Xpreprocessor) because we either integrate their functionality // (assembler and preprocessor), or bypass a previous driver ('collect2'). @@ -1783,7 +1909,7 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) { } // Perform the default argument translations. - DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs); + DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs, TC.getTriple()); // Check if the environment version is valid except wasm case. llvm::Triple Triple = TC.getTriple(); diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp index 25ea64a4ba5055..d32d5b9a52fc94 100644 --- a/clang/lib/Driver/ToolChains/MSVC.cpp +++ b/clang/lib/Driver/ToolChains/MSVC.cpp @@ -1023,103 +1023,6 @@ SanitizerMask MSVCToolChain::getSupportedSanitizers( return Res; } -static void TranslateOptArg(Arg *A, llvm::opt::DerivedArgList &DAL, - bool SupportsForcingFramePointer, - const char *ExpandChar, const OptTable &Opts) { - assert(A->getOption().matches(options::OPT__SLASH_O)); - - StringRef OptStr = A->getValue(); - for (size_t I = 0, E = OptStr.size(); I != E; ++I) { - const char &OptChar = *(OptStr.data() + I); - switch (OptChar) { - default: - break; - case '1': - case '2': - case 'x': - case 'd': - // Ignore /O[12xd] flags that aren't the last one on the command line. - // Only the last one gets expanded. - if (&OptChar != ExpandChar) { - A->claim(); - break; - } - if (OptChar == 'd') { - DAL.AddFlagArg(A, Opts.getOption(options::OPT_O0)); - } else { - if (OptChar == '1') { - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); - } else if (OptChar == '2' || OptChar == 'x') { - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); - } - if (SupportsForcingFramePointer && - !DAL.hasArgNoClaim(options::OPT_fno_omit_frame_pointer)) - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); - if (OptChar == '1' || OptChar == '2') - DAL.AddFlagArg(A, Opts.getOption(options::OPT_ffunction_sections)); - } - break; - case 'b': - if (I + 1 != E && isdigit(OptStr[I + 1])) { - switch (OptStr[I + 1]) { - case '0': - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_inline)); - break; - case '1': - DAL.AddFlagArg(A, Opts.getOption(options::OPT_finline_hint_functions)); - break; - case '2': - case '3': - DAL.AddFlagArg(A, Opts.getOption(options::OPT_finline_functions)); - break; - } - ++I; - } - break; - case 'g': - A->claim(); - break; - case 'i': - if (I + 1 != E && OptStr[I + 1] == '-') { - ++I; - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_builtin)); - } else { - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); - } - break; - case 's': - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); - break; - case 't': - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); - break; - case 'y': { - bool OmitFramePointer = true; - if (I + 1 != E && OptStr[I + 1] == '-') { - OmitFramePointer = false; - ++I; - } - if (SupportsForcingFramePointer) { - if (OmitFramePointer) - DAL.AddFlagArg(A, - Opts.getOption(options::OPT_fomit_frame_pointer)); - else - DAL.AddFlagArg( - A, Opts.getOption(options::OPT_fno_omit_frame_pointer)); - } else { - // Don't warn about /Oy- in x86-64 builds (where - // SupportsForcingFramePointer is false). The flag having no effect - // there is a compiler-internal optimization, and people shouldn't have - // to special-case their build files for x86-64 clang-cl. - A->claim(); - } - break; - } - } - } -} - static void TranslateDArg(Arg *A, llvm::opt::DerivedArgList &DAL, const OptTable &Opts) { assert(A->getOption().matches(options::OPT_D)); @@ -1154,39 +1057,8 @@ MSVCToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs()); const OptTable &Opts = getDriver().getOpts(); - // /Oy and /Oy- don't have an effect on X86-64 - bool SupportsForcingFramePointer = getArch() != llvm::Triple::x86_64; - - // The -O[12xd] flag actually expands to several flags. We must desugar the - // flags so that options embedded can be negated. For example, the '-O2' flag - // enables '-Oy'. Expanding '-O2' into its constituent flags allows us to - // correctly handle '-O2 -Oy-' where the trailing '-Oy-' disables a single - // aspect of '-O2'. - // - // Note that this expansion logic only applies to the *last* of '[12xd]'. - - // First step is to search for the character we'd like to expand. - const char *ExpandChar = nullptr; - for (Arg *A : Args.filtered(options::OPT__SLASH_O)) { - StringRef OptStr = A->getValue(); - for (size_t I = 0, E = OptStr.size(); I != E; ++I) { - char OptChar = OptStr[I]; - char PrevChar = I > 0 ? OptStr[I - 1] : '0'; - if (PrevChar == 'b') { - // OptChar does not expand; it's an argument to the previous char. - continue; - } - if (OptChar == '1' || OptChar == '2' || OptChar == 'x' || OptChar == 'd') - ExpandChar = OptStr.data() + I; - } - } - for (Arg *A : Args) { - if (A->getOption().matches(options::OPT__SLASH_O)) { - // The -O flag actually takes an amalgam of other options. For example, - // '/Ogyb2' is equivalent to '/Og' '/Oy' '/Ob2'. - TranslateOptArg(A, *DAL, SupportsForcingFramePointer, ExpandChar, Opts); - } else if (A->getOption().matches(options::OPT_D)) { + if (A->getOption().matches(options::OPT_D)) { // Translate -Dfoo#bar into -Dfoo=bar. TranslateDArg(A, *DAL, Opts); } else if (A->getOption().matches(options::OPT__SLASH_permissive)) { diff --git a/clang/test/Driver/cl-options.cu b/clang/test/Driver/cl-options.cu index b241ec6672d851..2517e7d77d4a9e 100644 --- a/clang/test/Driver/cl-options.cu +++ b/clang/test/Driver/cl-options.cu @@ -25,3 +25,10 @@ // Gd-NOT: "-fdefault-calling-conv=cdecl" // Gd: "-cc1" "-triple" // Gd: "-fdefault-calling-conv=cdecl" + +// Optimization options must continue to apply to both CUDA compilation jobs. +// RUN: not %clang_cl /c /O2 -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck -check-prefix=O2 %s +// O2: "-cc1" "-triple" "nvptx{{(64)?}}-nvidia-cuda" +// O2-SAME: "-O3" +// O2: "-cc1" "-triple" +// O2-SAME: "-O3" diff --git a/clang/test/Driver/hip-cl-optimization.hip b/clang/test/Driver/hip-cl-optimization.hip new file mode 100644 index 00000000000000..3a3720f0a5c67f --- /dev/null +++ b/clang/test/Driver/hip-cl-optimization.hip @@ -0,0 +1,106 @@ +// Check that clang-cl optimization options apply to both host and device. +// Device-specific options must still override the shared optimization level. + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O1 -- %s 2>&1 | FileCheck %s --check-prefix=OS + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 -- %s 2>&1 | FileCheck %s --check-prefix=O3 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /Ox -- %s 2>&1 | FileCheck %s --check-prefix=O3 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /Od -- %s 2>&1 | FileCheck %s --check-prefix=O0 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 /Od -- %s 2>&1 | FileCheck %s --check-prefix=O0 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /Od /O2 -- %s 2>&1 | FileCheck %s --check-prefix=O3 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 -clang:-O1 -- %s 2>&1 | FileCheck %s --check-prefix=O1 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 -clang:-Xarch_device -clang:-O1 -- %s 2>&1 | FileCheck %s --check-prefix=DEVICE-O1 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 /Ob0 -- %s 2>&1 | FileCheck %s --check-prefix=NOINLINE + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O1 -- %s 2>&1 | FileCheck %s --check-prefix=OS + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 -- %s 2>&1 | FileCheck %s --check-prefix=O3 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /Ox -- %s 2>&1 | FileCheck %s --check-prefix=O3 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /Od -- %s 2>&1 | FileCheck %s --check-prefix=O0 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 /Od -- %s 2>&1 | FileCheck %s --check-prefix=O0 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /Od /O2 -- %s 2>&1 | FileCheck %s --check-prefix=O3 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 -clang:-O1 -- %s 2>&1 | FileCheck %s --check-prefix=O1 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 -clang:-Xarch_device -clang:-O1 -- %s 2>&1 | FileCheck %s --check-prefix=DEVICE-O1 + +// RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O2 /Ob0 -- %s 2>&1 | FileCheck %s --check-prefix=NOINLINE + +// OS: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// OS-SAME: "-Os" +// OS: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" +// OS-SAME: "-Os" + +// O3: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// O3-SAME: "-O3" +// O3: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" +// O3-SAME: "-O3" + +// O0: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// O0-SAME: "-O0" +// O0: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" +// O0-SAME: "-O0" + +// O1: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// O1-SAME: "-O1" +// O1: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" +// O1-SAME: "-O1" + +// DEVICE-O1: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// DEVICE-O1-SAME: "-O1" +// DEVICE-O1: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" +// DEVICE-O1-SAME: "-O3" + +// NOINLINE: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// NOINLINE-SAME: "-O3" +// NOINLINE-SAME: "-fno-inline" +// NOINLINE: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" +// NOINLINE-SAME: "-O3" +// NOINLINE-SAME: "-fno-inline" >From c437113e897dce7106e7a3fa7ca21dbd8e19b781 Mon Sep 17 00:00:00 2001 From: "Yaxun (Sam) Liu" <[email protected]> Date: Fri, 2 Oct 2026 10:21:12 -0400 Subject: [PATCH 2/2] [clang] Extract shared clang-cl optimization translation Keep clang-cl optimization parsing in an internal helper while the driver normalizes options before host and device arguments are split. Select translation by clang-cl mode so Linux and MinGW host toolchains also receive the canonical optimization options. --- clang/docs/ReleaseNotes.md | 3 +- clang/lib/Driver/CMakeLists.txt | 1 + clang/lib/Driver/ClangCLArgs.cpp | 139 ++++++++++++++++++++++ clang/lib/Driver/ClangCLArgs.h | 41 +++++++ clang/lib/Driver/Driver.cpp | 127 +------------------- clang/test/Driver/cl-options.c | 9 ++ clang/test/Driver/hip-cl-optimization.hip | 13 ++ 7 files changed, 210 insertions(+), 123 deletions(-) create mode 100644 clang/lib/Driver/ClangCLArgs.cpp create mode 100644 clang/lib/Driver/ClangCLArgs.h diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 4fdefafccea14d..4d46ed3b646afc 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -816,7 +816,8 @@ features cannot lower the translation-unit ABI level; #### Miscellaneous Bug Fixes - Fixed `clang-cl` optimization options such as `/O1`, `/O2`, and - `/Ox` being ignored when compiling HIP device code. + `/Ox` being ignored when compiling HIP device code or using a non-MSVC + host toolchain. #### Miscellaneous Clang Crashes Fixed diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt index 506536cdc04f5f..c33dfd2471f35a 100644 --- a/clang/lib/Driver/CMakeLists.txt +++ b/clang/lib/Driver/CMakeLists.txt @@ -24,6 +24,7 @@ endif() add_clang_library(clangDriver Action.cpp + ClangCLArgs.cpp Compilation.cpp CreateASTUnitFromArgs.cpp CreateInvocationFromArgs.cpp diff --git a/clang/lib/Driver/ClangCLArgs.cpp b/clang/lib/Driver/ClangCLArgs.cpp new file mode 100644 index 00000000000000..64ff6adefd2567 --- /dev/null +++ b/clang/lib/Driver/ClangCLArgs.cpp @@ -0,0 +1,139 @@ +//===--- ClangCLArgs.cpp - clang-cl arguments -----------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ClangCLArgs.h" +#include "clang/Options/Options.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Option/Arg.h" +#include "llvm/Option/ArgList.h" +#include "llvm/Option/OptTable.h" +#include "llvm/Option/Option.h" +#include "llvm/TargetParser/Triple.h" +#include <cstddef> + +using namespace clang; +using namespace clang::driver; +using namespace llvm::opt; + +ClangCLArgs::ClangCLArgs(const ArgList &Args, const llvm::Triple &HostTriple) + : SupportsForcingFramePointer(HostTriple.getArch() != + llvm::Triple::x86_64) { + // Expand only the last /O[12xd], preserving overrides such as /O2 /Oy-. + for (Arg *A : Args.filtered(options::OPT__SLASH_O)) { + llvm::StringRef OptStr = A->getValue(); + for (size_t I = 0, E = OptStr.size(); I != E; ++I) { + char OptChar = OptStr[I]; + char PrevChar = I > 0 ? OptStr[I - 1] : '0'; + if (PrevChar == 'b') { + // OptChar does not expand; it's an argument to the previous char. + continue; + } + if (OptChar == '1' || OptChar == '2' || OptChar == 'x' || OptChar == 'd') + ExpandChar = OptStr.data() + I; + } + } +} + +bool ClangCLArgs::translateArg(Arg *A, DerivedArgList &DAL) const { + if (!A->getOption().matches(options::OPT__SLASH_O)) + return false; + + // Keep the original argument for unused-option diagnostics. + DAL.append(A); + const OptTable &Opts = getDriverOptTable(); + + llvm::StringRef OptStr = A->getValue(); + for (size_t I = 0, E = OptStr.size(); I != E; ++I) { + const char &OptChar = *(OptStr.data() + I); + switch (OptChar) { + default: + break; + case '1': + case '2': + case 'x': + case 'd': + // Ignore /O[12xd] flags that aren't the last one on the command line. + // Only the last one gets expanded. + if (&OptChar != ExpandChar) { + A->claim(); + break; + } + if (OptChar == 'd') { + DAL.AddFlagArg(A, Opts.getOption(options::OPT_O0)); + } else { + if (OptChar == '1') { + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); + } else if (OptChar == '2' || OptChar == 'x') { + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); + } + if (SupportsForcingFramePointer && + !DAL.hasArgNoClaim(options::OPT_fno_omit_frame_pointer)) + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); + if (OptChar == '1' || OptChar == '2') + DAL.AddFlagArg(A, Opts.getOption(options::OPT_ffunction_sections)); + } + break; + case 'b': + if (I + 1 != E && llvm::isDigit(OptStr[I + 1])) { + switch (OptStr[I + 1]) { + case '0': + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_inline)); + break; + case '1': + DAL.AddFlagArg(A, + Opts.getOption(options::OPT_finline_hint_functions)); + break; + case '2': + case '3': + DAL.AddFlagArg(A, Opts.getOption(options::OPT_finline_functions)); + break; + } + ++I; + } + break; + case 'g': + A->claim(); + break; + case 'i': + if (I + 1 != E && OptStr[I + 1] == '-') { + ++I; + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_builtin)); + } else { + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); + } + break; + case 's': + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); + break; + case 't': + DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); + break; + case 'y': { + bool OmitFramePointer = true; + if (I + 1 != E && OptStr[I + 1] == '-') { + OmitFramePointer = false; + ++I; + } + if (SupportsForcingFramePointer) { + if (OmitFramePointer) + DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); + else + DAL.AddFlagArg(A, + Opts.getOption(options::OPT_fno_omit_frame_pointer)); + } else { + // Silently accept /Oy- on x86-64 for portable clang-cl build flags. + A->claim(); + } + break; + } + } + } + return true; +} diff --git a/clang/lib/Driver/ClangCLArgs.h b/clang/lib/Driver/ClangCLArgs.h new file mode 100644 index 00000000000000..fbfd24064fa814 --- /dev/null +++ b/clang/lib/Driver/ClangCLArgs.h @@ -0,0 +1,41 @@ +//===--- ClangCLArgs.h - clang-cl arguments ---------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_DRIVER_CLANGCLARGS_H +#define LLVM_CLANG_LIB_DRIVER_CLANGCLARGS_H + +#include "llvm/Support/Compiler.h" + +namespace llvm { +class Triple; +namespace opt { +class Arg; +class ArgList; +class DerivedArgList; +} // namespace opt +} // namespace llvm + +namespace clang::driver { + +/// Translate clang-cl options before host and device arguments are split. +class LLVM_LIBRARY_VISIBILITY ClangCLArgs { + const char *ExpandChar = nullptr; + bool SupportsForcingFramePointer; + +public: + /// The argument values must outlive this translator. + ClangCLArgs(const llvm::opt::ArgList &Args, const llvm::Triple &HostTriple); + + /// Append A and its canonical expansion to DAL. Return false without + /// modifying DAL if A is not handled. + bool translateArg(llvm::opt::Arg *A, llvm::opt::DerivedArgList &DAL) const; +}; + +} // namespace clang::driver + +#endif // LLVM_CLANG_LIB_DRIVER_CLANGCLARGS_H diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 07f70fbbe0128c..20d5520fa03b3d 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "clang/Driver/Driver.h" +#include "ClangCLArgs.h" #include "ToolChains/AIX.h" #include "ToolChains/AMDGPU.h" #include "ToolChains/AVR.h" @@ -459,127 +460,14 @@ Arg *clang::driver::makeInputArg(DerivedArgList &Args, const OptTable &Opts, return A; } -static void translateMSVCOptArg(Arg *A, llvm::opt::DerivedArgList &DAL, - bool SupportsForcingFramePointer, - const char *ExpandChar, const OptTable &Opts) { - assert(A->getOption().matches(options::OPT__SLASH_O)); - - StringRef OptStr = A->getValue(); - for (size_t I = 0, E = OptStr.size(); I != E; ++I) { - const char &OptChar = *(OptStr.data() + I); - switch (OptChar) { - default: - break; - case '1': - case '2': - case 'x': - case 'd': - // Ignore /O[12xd] flags that aren't the last one on the command line. - // Only the last one gets expanded. - if (&OptChar != ExpandChar) { - A->claim(); - break; - } - if (OptChar == 'd') { - DAL.AddFlagArg(A, Opts.getOption(options::OPT_O0)); - } else { - if (OptChar == '1') { - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); - } else if (OptChar == '2' || OptChar == 'x') { - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); - } - if (SupportsForcingFramePointer && - !DAL.hasArgNoClaim(options::OPT_fno_omit_frame_pointer)) - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); - if (OptChar == '1' || OptChar == '2') - DAL.AddFlagArg(A, Opts.getOption(options::OPT_ffunction_sections)); - } - break; - case 'b': - if (I + 1 != E && isdigit(OptStr[I + 1])) { - switch (OptStr[I + 1]) { - case '0': - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_inline)); - break; - case '1': - DAL.AddFlagArg(A, - Opts.getOption(options::OPT_finline_hint_functions)); - break; - case '2': - case '3': - DAL.AddFlagArg(A, Opts.getOption(options::OPT_finline_functions)); - break; - } - ++I; - } - break; - case 'g': - A->claim(); - break; - case 'i': - if (I + 1 != E && OptStr[I + 1] == '-') { - ++I; - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_builtin)); - } else { - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fbuiltin)); - } - break; - case 's': - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "s"); - break; - case 't': - DAL.AddJoinedArg(A, Opts.getOption(options::OPT_O), "3"); - break; - case 'y': { - bool OmitFramePointer = true; - if (I + 1 != E && OptStr[I + 1] == '-') { - OmitFramePointer = false; - ++I; - } - if (SupportsForcingFramePointer) { - if (OmitFramePointer) - DAL.AddFlagArg(A, Opts.getOption(options::OPT_fomit_frame_pointer)); - else - DAL.AddFlagArg(A, - Opts.getOption(options::OPT_fno_omit_frame_pointer)); - } else { - // Silently accept /Oy- on x86-64 for portable clang-cl build flags. - A->claim(); - } - break; - } - } - } -} - DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args, const llvm::Triple &Triple) const { const llvm::opt::OptTable &Opts = getOpts(); DerivedArgList *DAL = new DerivedArgList(Args); - // Normalize MSVC optimization options before host and device arguments split. - bool TranslateMSVCOpts = Triple.isWindowsMSVCEnvironment(); - // /Oy and /Oy- do not affect the x86-64 host. - bool SupportsForcingFramePointer = Triple.getArch() != llvm::Triple::x86_64; - // Expand only the last /O[12xd], preserving overrides such as /O2 /Oy-. - const char *ExpandChar = nullptr; - if (TranslateMSVCOpts) { - for (Arg *A : Args.filtered(options::OPT__SLASH_O)) { - StringRef OptStr = A->getValue(); - for (size_t I = 0, E = OptStr.size(); I != E; ++I) { - char OptChar = OptStr[I]; - char PrevChar = I > 0 ? OptStr[I - 1] : '0'; - if (PrevChar == 'b') { - // OptChar does not expand; it's an argument to the previous char. - continue; - } - if (OptChar == '1' || OptChar == '2' || OptChar == 'x' || - OptChar == 'd') - ExpandChar = OptStr.data() + I; - } - } - } + std::optional<ClangCLArgs> CLArgs; + if (IsCLMode()) + CLArgs.emplace(Args, Triple); bool HasNostdlib = Args.hasArg(options::OPT_nostdlib); bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx); @@ -598,13 +486,8 @@ DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args, continue; } - if (TranslateMSVCOpts && A->getOption().matches(options::OPT__SLASH_O)) { - // Keep the original argument for unused-option diagnostics. - DAL->append(A); - translateMSVCOptArg(A, *DAL, SupportsForcingFramePointer, ExpandChar, - Opts); + if (CLArgs && CLArgs->translateArg(A, *DAL)) continue; - } // Unfortunately, we have to parse some forwarding options (-Xassembler, // -Xlinker, -Xpreprocessor) because we either integrate their functionality diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c index 57d82622ef4de6..b1d0ccbf60f0de 100644 --- a/clang/test/Driver/cl-options.c +++ b/clang/test/Driver/cl-options.c @@ -184,6 +184,15 @@ // RUN: %clang_cl /Od -### -- %s 2>&1 | FileCheck -check-prefix=Od %s // Od: -O0 +// clang-cl optimization options are independent of the host toolchain. +// RUN: %clang_cl --target=x86_64-unknown-linux-gnu /O1 -### -- %s 2>&1 | FileCheck %s --check-prefix=CL-LINUX-OS +// CL-LINUX-OS: "-cc1" +// CL-LINUX-OS-SAME: "-Os" + +// RUN: %clang_cl --target=x86_64-w64-windows-gnu /O2 -### -- %s 2>&1 | FileCheck %s --check-prefix=CL-MINGW-O3 +// CL-MINGW-O3: "-cc1" +// CL-MINGW-O3-SAME: "-O3" + // RUN: %clang_cl /Oi- /Oi -### -- %s 2>&1 | FileCheck -check-prefix=Oi %s // Oi-NOT: -fno-builtin diff --git a/clang/test/Driver/hip-cl-optimization.hip b/clang/test/Driver/hip-cl-optimization.hip index 3a3720f0a5c67f..1fa80de854e566 100644 --- a/clang/test/Driver/hip-cl-optimization.hip +++ b/clang/test/Driver/hip-cl-optimization.hip @@ -5,6 +5,10 @@ // RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ // RUN: /O1 -- %s 2>&1 | FileCheck %s --check-prefix=OS +// RUN: %clang_cl --target=x86_64-unknown-linux-gnu -### /c --offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O1 -- %s 2>&1 | FileCheck %s --check-prefix=LINUX + // RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --offload-new-driver \ // RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ // RUN: /O2 -- %s 2>&1 | FileCheck %s --check-prefix=O3 @@ -41,6 +45,10 @@ // RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ // RUN: /O1 -- %s 2>&1 | FileCheck %s --check-prefix=OS +// RUN: %clang_cl --target=x86_64-unknown-linux-gnu -### /c --no-offload-new-driver \ +// RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ +// RUN: /O1 -- %s 2>&1 | FileCheck %s --check-prefix=LINUX + // RUN: %clang_cl --target=x86_64-pc-windows-msvc -### /c --no-offload-new-driver \ // RUN: -x hip --offload-arch=gfx1100 -clang:-nogpuinc -clang:-nogpulib \ // RUN: /O2 -- %s 2>&1 | FileCheck %s --check-prefix=O3 @@ -78,6 +86,11 @@ // OS: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" // OS-SAME: "-Os" +// LINUX: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" +// LINUX-SAME: "-Os" +// LINUX: "-cc1" "-triple" "x86_64-unknown-linux-gnu" +// LINUX-SAME: "-Os" + // O3: "-cc1" "-triple" "amdgpu11.00-amd-amdhsa" // O3-SAME: "-O3" // O3: "-cc1" "-triple" "x86_64-pc-windows-msvc{{[^"]*}}" _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
