-
Notifications
You must be signed in to change notification settings - Fork 738
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
Conversation
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.
@@ -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 \ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
.
clang/lib/Driver/Driver.cpp
Outdated
StringRef ArgValue(DeviceObj->getValue()); | ||
SmallVector<StringRef, 2> DeviceObjValues = {"spirv", "llvmir"}; | ||
auto FoundArg = | ||
std::find(DeviceObjValues.begin(), DeviceObjValues.end(), ArgValue); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
Testing issue is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending fix of typo.
FE change just adds a diagnostic
@intel/llvm-gatekeepers, this PR looks ready for merge - please take a look. Thanks! |
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.