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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Sep 16, 2024
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1700c40
[Driver][SYCL] Add diagnostic for bad argument with -fsycl-device-obj
mdtoguchi Sep 12, 2024
15d8d94
Simplify logic
mdtoguchi Sep 12, 2024
d745f82
Fix comment typo
mdtoguchi Sep 13, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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