From 1700c409ae16ad1b6c1079b1c270c07761c58074 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Thu, 12 Sep 2024 11:02:03 -0700 Subject: [PATCH 1/3] [Driver][SYCL] Add diagnostic for bad argument with -fsycl-device-obj When using -fsycl-device-obj, we would effectively ignore any bad arguments and default to 'llvmir'. Add a diagnostic to inform the user of the bad argument and what we are doing with our default behavior. --- .../include/clang/Basic/DiagnosticDriverKinds.td | 3 +++ clang/lib/Driver/Driver.cpp | 15 +++++++++++++++ clang/test/Driver/sycl-offload.c | 7 +++++++ 3 files changed, 25 insertions(+) diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index b6e71082be66d..0428079fc79ad 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -520,6 +520,9 @@ def warn_drv_no_floating_point_registers: Warning< InGroup; def warn_ignoring_ftabstop_value : Warning< "ignoring invalid -ftabstop value '%0', using default value %1">; +def warn_ignoring_value_using_default : Warning< + "ignoring invalid '%0' value '%1', using default value '%2'">, + InGroup; def warn_drv_overriding_option : Warning< "overriding '%0' option with '%1'">, InGroup>; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index c888733f1d885..f674e7c0970bf 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1169,6 +1169,21 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C, C.getInputArgs().getLastArg(options::OPT_fsycl_range_rounding_EQ); checkSingleArgValidity(RangeRoundingPreference, {"disable", "force", "on"}); + // Evaluation of -fsycl-device-obj is slightly different, we will emit + // a warning and inform the user of the default behavior used. + // TODO: General usage of this option is to check fo 'spirv' and fallthrough + // to using llvmir. This can be improved to be more obvious in usage. + if (Arg *DeviceObj = C.getInputArgs().getLastArgNoClaim( + options::OPT_fsycl_device_obj_EQ)) { + StringRef ArgValue(DeviceObj->getValue()); + SmallVector DeviceObjValues = {"spirv", "llvmir"}; + auto FoundArg = + std::find(DeviceObjValues.begin(), DeviceObjValues.end(), ArgValue); + if (FoundArg == DeviceObjValues.end()) + Diag(clang::diag::warn_ignoring_value_using_default) + << DeviceObj->getSpelling().split('=').first << ArgValue << "llvmir"; + } + Arg *SYCLForceTarget = getArgRequiringSYCLRuntime(options::OPT_fsycl_force_target_EQ); if (SYCLForceTarget) { diff --git a/clang/test/Driver/sycl-offload.c b/clang/test/Driver/sycl-offload.c index 60db00bea9c4f..17602ad2ed614 100644 --- a/clang/test/Driver/sycl-offload.c +++ b/clang/test/Driver/sycl-offload.c @@ -69,6 +69,13 @@ // RUN: | FileCheck -check-prefix=CHK-SYCL-TARGET %s // CHK-SYCL-TARGET-NOT: error: SYCL target is invalid +/// -fsycl-device-obj argument check +// RUN: %clang -### -fsycl-device-obj=test -fsycl %s 2>&1 \ +// RUN: | FileCheck -check-prefix=DEVICE_OBJ_WARN %s +// RUN: %clang_cl -### -fsycl-device-obj=test -fsycl %s 2>&1 \ +// RUN: | FileCheck -check-prefix=DEVICE_OBJ_WARN %s +// DEVICE_OBJ_WARN: warning: ignoring invalid '-fsycl-device-obj' value 'test', using default value 'llvmir' [-Wunused-command-line-argument] + /// ########################################################################### /// Check warning for duplicate offloading targets. From 15d8d945532eac32b29b273c23a15732065b2abc Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Thu, 12 Sep 2024 14:00:01 -0700 Subject: [PATCH 2/3] Simplify logic --- clang/lib/Driver/Driver.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index f674e7c0970bf..e303b36386cee 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1177,9 +1177,7 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C, options::OPT_fsycl_device_obj_EQ)) { StringRef ArgValue(DeviceObj->getValue()); SmallVector DeviceObjValues = {"spirv", "llvmir"}; - auto FoundArg = - std::find(DeviceObjValues.begin(), DeviceObjValues.end(), ArgValue); - if (FoundArg == DeviceObjValues.end()) + if (llvm::find(DeviceObjValues, ArgValue) == DeviceObjValues.end()) Diag(clang::diag::warn_ignoring_value_using_default) << DeviceObj->getSpelling().split('=').first << ArgValue << "llvmir"; } From d745f823bb371b4b83ce00b8f4c1eeeff21a1b2e Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Fri, 13 Sep 2024 11:20:23 -0700 Subject: [PATCH 3/3] Fix comment typo --- clang/lib/Driver/Driver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e303b36386cee..047f712aa0d3b 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -1171,7 +1171,7 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C, // Evaluation of -fsycl-device-obj is slightly different, we will emit // a warning and inform the user of the default behavior used. - // TODO: General usage of this option is to check fo 'spirv' and fallthrough + // TODO: General usage of this option is to check for 'spirv' and fallthrough // to using llvmir. This can be improved to be more obvious in usage. if (Arg *DeviceObj = C.getInputArgs().getLastArgNoClaim( options::OPT_fsycl_device_obj_EQ)) {