From ced00e8420ee5dd6bfc765bd3ca30cdd423b0045 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 17 Mar 2021 06:29:50 -0700 Subject: [PATCH 1/2] Update the way we handle -sycl-std= based on community review feedback When upstreaming these changes, community asked for some simple modifications in https://reviews.llvm.org/D97717. This brings those changes back downstream for parity. --- clang/include/clang/Driver/Options.td | 13 +++++++---- .../Frontend/CompilerInvocationTest.cpp | 22 +++++++------------ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 868453b1b8c93..af30806d3001f 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4295,10 +4295,6 @@ def fsycl : Flag<["-"], "fsycl">, Flags<[NoXarchOption, CoreOption]>, Group; def fno_sycl : Flag<["-"], "fno-sycl">, Flags<[NoXarchOption, CoreOption]>, Group, HelpText<"Disables SYCL kernels compilation for device">; -def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, Flags<[CC1Option, NoArgumentUnused, CoreOption]>, - HelpText<"SYCL language standard to compile for.">, Values<"2020,2017,121,1.2.1,sycl-1.2.1">, - NormalizedValues<["SYCL_2020", "SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>, NormalizedValuesScope<"LangOptions">, - MarshallingInfoString, "SYCL_None">, AutoNormalizeEnum; defm sycl_esimd: BoolFOption<"sycl-explicit-simd", LangOpts<"SYCLExplicitSIMD">, DefaultFalse, PosFlag, NegFlag, @@ -5520,6 +5516,15 @@ def fsycl_is_device : Flag<["-"], "fsycl-is-device">, def fsycl_is_host : Flag<["-"], "fsycl-is-host">, HelpText<"SYCL host compilation">, MarshallingInfoFlag>; +def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, + Flags<[CC1Option, NoArgumentUnused, CoreOption]>, + HelpText<"SYCL language standard to compile for.">, + Values<"2020,2017,121,1.2.1,sycl-1.2.1">, + NormalizedValues<["SYCL_2020", "SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>, + NormalizedValuesScope<"LangOptions">, + MarshallingInfoString, "SYCL_None">, + AutoNormalizeEnum, + ShouldParseIf; def fsycl_int_header : Separate<["-"], "fsycl-int-header">, HelpText<"Generate SYCL integration header into this file.">, MarshallingInfoString>; diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp index 811e9414345fb..9d921c833d19b 100644 --- a/clang/unittests/Frontend/CompilerInvocationTest.cpp +++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp @@ -531,46 +531,40 @@ TEST_F(CommandLineTest, ConditionalParsingIfFalseFlagPresent) { ASSERT_FALSE(Diags->hasErrorOccurred()); ASSERT_FALSE(Invocation.getLangOpts()->SYCLIsDevice); ASSERT_FALSE(Invocation.getLangOpts()->SYCLIsHost); - ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_2017); + ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_None); Invocation.generateCC1CommandLine(GeneratedArgs, *this); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl-is-device")))); ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl-is-host")))); - - // FIXME: generateCC1CommandLine is only used by the unit test system and - // cannot handle this case. It passes along the -sycl-std because the option - // definition does not specify that it relies on -fsycl any longer (because - // there is no syntax I could find that would allow it). However, the option - // is handled properly on a real invocation. See: Clang::ConstructJob(). - ASSERT_THAT(GeneratedArgs, Contains(HasSubstr("-sycl-std="))); + ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std=")))); } TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagNotPresent) { - const char *Args[] = {"-fsycl"}; + const char *Args[] = {"-fsycl-is-host"}; CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); - ASSERT_TRUE(Diags->hasErrorOccurred()); + ASSERT_FALSE(Diags->hasErrorOccurred()); ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_None); Invocation.generateCC1CommandLine(GeneratedArgs, *this); - ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl")))); + ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl-is-host"))); ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std=")))); } TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagPresent) { - const char *Args[] = {"-fsycl", "-sycl-std=2017"}; + const char *Args[] = {"-fsycl-is-device", "-sycl-std=2017"}; CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags); - ASSERT_TRUE(Diags->hasErrorOccurred()); + ASSERT_FALSE(Diags->hasErrorOccurred()); ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_2017); Invocation.generateCC1CommandLine(GeneratedArgs, *this); - ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl")))); + ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl-is-device"))); ASSERT_THAT(GeneratedArgs, Contains(StrEq("-sycl-std=2017"))); } From 72bd0cd949fe20a5ccef77b2fe816f02192b046b Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Wed, 17 Mar 2021 09:21:45 -0700 Subject: [PATCH 2/2] Correct build failures. Move the sycl-std= option out of a "let" block it shouldn't have been in. --- clang/include/clang/Driver/Options.td | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index af30806d3001f..9b4254f2c1dc4 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -5516,15 +5516,6 @@ def fsycl_is_device : Flag<["-"], "fsycl-is-device">, def fsycl_is_host : Flag<["-"], "fsycl-is-host">, HelpText<"SYCL host compilation">, MarshallingInfoFlag>; -def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, - Flags<[CC1Option, NoArgumentUnused, CoreOption]>, - HelpText<"SYCL language standard to compile for.">, - Values<"2020,2017,121,1.2.1,sycl-1.2.1">, - NormalizedValues<["SYCL_2020", "SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>, - NormalizedValuesScope<"LangOptions">, - MarshallingInfoString, "SYCL_None">, - AutoNormalizeEnum, - ShouldParseIf; def fsycl_int_header : Separate<["-"], "fsycl-int-header">, HelpText<"Generate SYCL integration header into this file.">, MarshallingInfoString>; @@ -5540,6 +5531,16 @@ def fenable_sycl_dae : Flag<["-"], "fenable-sycl-dae">, } // let Flags = [CC1Option, NoDriverOption] +def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group, + Flags<[CC1Option, NoArgumentUnused, CoreOption]>, + HelpText<"SYCL language standard to compile for.">, + Values<"2020,2017,121,1.2.1,sycl-1.2.1">, + NormalizedValues<["SYCL_2020", "SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>, + NormalizedValuesScope<"LangOptions">, + MarshallingInfoString, "SYCL_None">, + AutoNormalizeEnum, + ShouldParseIf; + defm cuda_approx_transcendentals : BoolFOption<"cuda-approx-transcendentals", LangOpts<"CUDADeviceApproxTranscendentals">, DefaultFalse, PosFlag, NegFlag,