From b7fb0a266522a4cf999a02d0b0659970d5efc4e5 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Fri, 18 Sep 2020 20:59:09 -0700 Subject: [PATCH 1/5] [Driver][SYCL] Adjust /MD settings for SYCL device compilations When setting /MD (by default) we do not want to pass the _MT and _DLL predefines to the device compilations, only the host for clang-cl --- clang/lib/Driver/ToolChains/Clang.cpp | 36 ++++++++++++++++----------- clang/lib/Driver/ToolChains/Clang.h | 4 +-- clang/test/Driver/sycl-MD-default.cpp | 1 + 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 532909e8a035..42a5754bbb79 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4941,7 +4941,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // Add clang-cl arguments. types::ID InputType = Input.getType(); if (D.IsCLMode()) - AddClangCLArgs(Args, InputType, CmdArgs, &DebugInfoKind, &EmitCodeView); + AddClangCLArgs(JA, Args, InputType, CmdArgs, &DebugInfoKind, &EmitCodeView); DwarfFissionKind DwarfFission; RenderDebugOptions(TC, D, RawTriple, Args, EmitCodeView, CmdArgs, @@ -6777,8 +6777,8 @@ static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args) { return EH; } -void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, - ArgStringList &CmdArgs, +void Clang::AddClangCLArgs(const JobAction &JA, const ArgList &Args, + types::ID InputType, ArgStringList &CmdArgs, codegenoptions::DebugInfoKind *DebugInfoKind, bool *EmitCodeView) const { unsigned RTOptionID = options::OPT__SLASH_MT; @@ -6804,31 +6804,37 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, << A->getOption().getName(); } + bool isSYCLDevice = Args.hasArg(options::OPT_fsycl_device_only) || + (JA.isDeviceOffloading(Action::OFK_SYCL) && + JA.isOffloading(Action::OFK_SYCL)); + enum { addDEBUG = 0x1, addMT = 0x2, addDLL = 0x4 }; + auto addPreDefines = [&](unsigned Defines) { + if (Defines & addDEBUG) + CmdArgs.push_back("-D_DEBUG"); + if (Defines & addMT && !isSYCLDevice) + CmdArgs.push_back("-D_MT"); + if (Defines & addDLL && !isSYCLDevice) + CmdArgs.push_back("-D_DLL"); + }; StringRef FlagForCRT; switch (RTOptionID) { case options::OPT__SLASH_MD: - if (Args.hasArg(options::OPT__SLASH_LDd)) - CmdArgs.push_back("-D_DEBUG"); - CmdArgs.push_back("-D_MT"); - CmdArgs.push_back("-D_DLL"); + addPreDefines((Args.hasArg(options::OPT__SLASH_LDd) ? addDEBUG : 0x0) | + addMT | addDLL); FlagForCRT = "--dependent-lib=msvcrt"; break; case options::OPT__SLASH_MDd: - CmdArgs.push_back("-D_DEBUG"); - CmdArgs.push_back("-D_MT"); - CmdArgs.push_back("-D_DLL"); + addPreDefines(addDEBUG | addMT | addDLL); FlagForCRT = "--dependent-lib=msvcrtd"; break; case options::OPT__SLASH_MT: - if (Args.hasArg(options::OPT__SLASH_LDd)) - CmdArgs.push_back("-D_DEBUG"); - CmdArgs.push_back("-D_MT"); + addPreDefines((Args.hasArg(options::OPT__SLASH_LDd) ? addDEBUG : 0x0) | + addMT); CmdArgs.push_back("-flto-visibility-public-std"); FlagForCRT = "--dependent-lib=libcmt"; break; case options::OPT__SLASH_MTd: - CmdArgs.push_back("-D_DEBUG"); - CmdArgs.push_back("-D_MT"); + addPreDefines(addDEBUG | addMT); CmdArgs.push_back("-flto-visibility-public-std"); FlagForCRT = "--dependent-lib=libcmtd"; break; diff --git a/clang/lib/Driver/ToolChains/Clang.h b/clang/lib/Driver/ToolChains/Clang.h index 939f53b04432..0ba3195c333d 100644 --- a/clang/lib/Driver/ToolChains/Clang.h +++ b/clang/lib/Driver/ToolChains/Clang.h @@ -83,8 +83,8 @@ class LLVM_LIBRARY_VISIBILITY Clang : public Tool { llvm::opt::ArgStringList &cmdArgs, RewriteKind rewrite) const; - void AddClangCLArgs(const llvm::opt::ArgList &Args, types::ID InputType, - llvm::opt::ArgStringList &CmdArgs, + void AddClangCLArgs(const JobAction &JA, const llvm::opt::ArgList &Args, + types::ID InputType, llvm::opt::ArgStringList &CmdArgs, codegenoptions::DebugInfoKind *DebugInfoKind, bool *EmitCodeView) const; diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index 5e88a48c76f5..5cad542db31d 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -10,6 +10,7 @@ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s // RUN: %clang_cl -### -MDd -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s +// CHK-DEFAULT-NOT: "-fsycl-is-device" {{.*}} "-D_MT" "-D_DLL" // CHK-DEFAULT: "-D_MT" "-D_DLL" // CHK-DEFAULT: "--dependent-lib=msvcrt{{d*}}" From 88aa142f7d2209a662645b6a3b8c95cd13ec2f9a Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Sat, 19 Sep 2020 06:10:32 -0700 Subject: [PATCH 2/5] [NFC] Do not emit the warning so many times, and clean up logic for SYCL device --- clang/lib/Driver/ToolChains/Clang.cpp | 17 +++++++---------- clang/lib/Driver/ToolChains/Clang.h | 4 ++-- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 42a5754bbb79..e6dad17b09c1 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4941,7 +4941,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // Add clang-cl arguments. types::ID InputType = Input.getType(); if (D.IsCLMode()) - AddClangCLArgs(JA, Args, InputType, CmdArgs, &DebugInfoKind, &EmitCodeView); + AddClangCLArgs(Args, InputType, CmdArgs, &DebugInfoKind, &EmitCodeView); DwarfFissionKind DwarfFission; RenderDebugOptions(TC, D, RawTriple, Args, EmitCodeView, CmdArgs, @@ -6777,15 +6777,15 @@ static EHFlags parseClangCLEHFlags(const Driver &D, const ArgList &Args) { return EH; } -void Clang::AddClangCLArgs(const JobAction &JA, const ArgList &Args, - types::ID InputType, ArgStringList &CmdArgs, +void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, + ArgStringList &CmdArgs, codegenoptions::DebugInfoKind *DebugInfoKind, bool *EmitCodeView) const { unsigned RTOptionID = options::OPT__SLASH_MT; bool isNVPTX = getToolChain().getTriple().isNVPTX(); - bool isSYCL = - Args.hasArg(options::OPT_fsycl) || + bool isSYCLDevice = getToolChain().getTriple().getEnvironment() == llvm::Triple::SYCLDevice; + bool isSYCL = Args.hasArg(options::OPT_fsycl) || isSYCLDevice; // For SYCL Windows, /MD is the default. if (isSYCL) RTOptionID = options::OPT__SLASH_MD; @@ -6797,16 +6797,13 @@ void Clang::AddClangCLArgs(const JobAction &JA, const ArgList &Args, if (Arg *A = Args.getLastArg(options::OPT__SLASH_M_Group)) { RTOptionID = A->getOption().getID(); - if (isSYCL && (RTOptionID == options::OPT__SLASH_MT || - RTOptionID == options::OPT__SLASH_MTd)) + if (isSYCL && !isSYCLDevice && (RTOptionID == options::OPT__SLASH_MT || + RTOptionID == options::OPT__SLASH_MTd)) // Use of /MT or /MTd is not supported for SYCL. getToolChain().getDriver().Diag(diag::err_drv_unsupported_opt_dpcpp) << A->getOption().getName(); } - bool isSYCLDevice = Args.hasArg(options::OPT_fsycl_device_only) || - (JA.isDeviceOffloading(Action::OFK_SYCL) && - JA.isOffloading(Action::OFK_SYCL)); enum { addDEBUG = 0x1, addMT = 0x2, addDLL = 0x4 }; auto addPreDefines = [&](unsigned Defines) { if (Defines & addDEBUG) diff --git a/clang/lib/Driver/ToolChains/Clang.h b/clang/lib/Driver/ToolChains/Clang.h index 0ba3195c333d..939f53b04432 100644 --- a/clang/lib/Driver/ToolChains/Clang.h +++ b/clang/lib/Driver/ToolChains/Clang.h @@ -83,8 +83,8 @@ class LLVM_LIBRARY_VISIBILITY Clang : public Tool { llvm::opt::ArgStringList &cmdArgs, RewriteKind rewrite) const; - void AddClangCLArgs(const JobAction &JA, const llvm::opt::ArgList &Args, - types::ID InputType, llvm::opt::ArgStringList &CmdArgs, + void AddClangCLArgs(const llvm::opt::ArgList &Args, types::ID InputType, + llvm::opt::ArgStringList &CmdArgs, codegenoptions::DebugInfoKind *DebugInfoKind, bool *EmitCodeView) const; From aac5592589cdf6de52280c235a29c7073c058a8a Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Sat, 19 Sep 2020 06:46:02 -0700 Subject: [PATCH 3/5] clang format --- clang/lib/Driver/ToolChains/Clang.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index e6dad17b09c1..68d0211308b7 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -6797,8 +6797,9 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, if (Arg *A = Args.getLastArg(options::OPT__SLASH_M_Group)) { RTOptionID = A->getOption().getID(); - if (isSYCL && !isSYCLDevice && (RTOptionID == options::OPT__SLASH_MT || - RTOptionID == options::OPT__SLASH_MTd)) + if (isSYCL && !isSYCLDevice && + (RTOptionID == options::OPT__SLASH_MT || + RTOptionID == options::OPT__SLASH_MTd)) // Use of /MT or /MTd is not supported for SYCL. getToolChain().getDriver().Diag(diag::err_drv_unsupported_opt_dpcpp) << A->getOption().getName(); From e768296fc285f509168f39fef75e84f2041d4d24 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 21 Sep 2020 12:14:48 -0700 Subject: [PATCH 4/5] [NFC] update test to clarify check for host compile. --- clang/test/Driver/sycl-MD-default.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index 5cad542db31d..853fd80161b3 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -11,8 +11,7 @@ // RUN: %clang_cl -### -MDd -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s // CHK-DEFAULT-NOT: "-fsycl-is-device" {{.*}} "-D_MT" "-D_DLL" -// CHK-DEFAULT: "-D_MT" "-D_DLL" -// CHK-DEFAULT: "--dependent-lib=msvcrt{{d*}}" +// CHK-DEFAULT: "-D_MT" "-D_DLL" "--dependent-lib=msvcrt{{d*}}" {{.*}} "-fsycl-is-host" // RUN: %clang_cl -### -MT -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-ERROR %s From 11aa94f325491045af0d9cd72f3c0d1a49854b0e Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 21 Sep 2020 13:21:36 -0700 Subject: [PATCH 5/5] [NFC] Fix test, clang output differs from clang-cl here --- clang/test/Driver/sycl-MD-default.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index 853fd80161b3..491745289c13 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -4,14 +4,17 @@ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s // RUN: %clangxx -### -fsycl -c -target x86_64-unknown-windows-msvc %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-DEFAULT %s +// CHK-DEFAULT-NOT: "-fsycl-is-device" {{.*}} "-D_MT" "-D_DLL" +// CHK-DEFAULT: "-fsycl-is-host" "-D_MT" "-D_DLL" "--dependent-lib=msvcrt{{d*}}" {{.*}} + // RUN: %clang_cl -### -fsycl -c %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHK-DEFAULT %s +// RUN: | FileCheck -check-prefix=CHK-DEFAULT-CL %s // RUN: %clang_cl -### -MD -fsycl -c %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHK-DEFAULT %s +// RUN: | FileCheck -check-prefix=CHK-DEFAULT-CL %s // RUN: %clang_cl -### -MDd -fsycl -c %s 2>&1 \ -// RUN: | FileCheck -check-prefix=CHK-DEFAULT %s -// CHK-DEFAULT-NOT: "-fsycl-is-device" {{.*}} "-D_MT" "-D_DLL" -// CHK-DEFAULT: "-D_MT" "-D_DLL" "--dependent-lib=msvcrt{{d*}}" {{.*}} "-fsycl-is-host" +// RUN: | FileCheck -check-prefix=CHK-DEFAULT-CL %s +// CHK-DEFAULT-CL-NOT: "-fsycl-is-device" {{.*}} "-D_MT" "-D_DLL" +// CHK-DEFAULT-CL: "-D_MT" "-D_DLL" "--dependent-lib=msvcrt{{d*}}" {{.*}} "-fsycl-is-host" // RUN: %clang_cl -### -MT -fsycl -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-ERROR %s