Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Driver][SYCL] Add diagnostic for bad argument with -fsycl-device-obj #15381

Merged
merged 3 commits into from
Sep 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,9 @@ def warn_drv_no_floating_point_registers: Warning<
InGroup<UnsupportedABI>;
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<UnusedCommandLineArgument>;
def warn_drv_overriding_option : Warning<
"overriding '%0' option with '%1'">,
InGroup<DiagGroup<"overriding-option">>;
Expand Down
15 changes: 15 additions & 0 deletions clang/lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
mdtoguchi marked this conversation as resolved.
Show resolved Hide resolved
// 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<StringRef, 2> DeviceObjValues = {"spirv", "llvmir"};
auto FoundArg =
std::find(DeviceObjValues.begin(), DeviceObjValues.end(), ArgValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a vector is used here, llvm::find_if can be used to instead of std::find and an if to simplify this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've simplified by using llvm::find (I found find_if to be just as 'complex' as the original)

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) {
Expand Down
7 changes: 7 additions & 0 deletions clang/test/Driver/sycl-offload.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the behavior when --offload-new-driver or --no-offload-new-driver is passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior is the same. The use of --offload-new-driver vs --no-offload-new-driver does not change the need for the diagnostic or the behavior involved with -fsycl-device-obj.

// 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.
Expand Down
Loading