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

[SYCL][Driver] Fix autodetection of the -Xsycl-target-backend triple #4463

Merged
merged 5 commits into from
Sep 6, 2021

Conversation

AGindinson
Copy link
Contributor

@AGindinson AGindinson commented Sep 2, 2021

#4175 introduced automatic addition
of the generic spir64 device target when any section of the input
objects had this triple assigned to it. As a result, the actual list
of toolchains started exceeding the user-provided one by 1 item.

After #4239, the above became a
problem. The dispatch of -Xsycl-target-* arguments started happening
earlier in theflow, which broke the following use-case:

clang++ -fsycl -fsycl-targets=spir64_gen gen-obj.o
gen-and-spir64-obj.o -Xsycl-target-backend "-device *"

The fix is to hold off the autodetected generic triple addition until the
GPU architectures have been set based off -Xsycl-target-backend --offload-arch= values.

Signed-off-by: Artem Gindinson artem.gindinson@intel.com

Artem Gindinson added 3 commits September 2, 2021 12:53
intel#4175 introduced automatic addition
of the generic spir64 device target when any section of the input
objects had this triple assigned to it. As a result, the actual list
of toolchains started exceeding the user-provided one by 1 item.

After intel#4239, the above became a
problem. The dispatch of -Xsycl-target-* arguments started happening
earlier in theflow, which broke the following use-case:
```
clang++ -fsycl -fsycl-targets=spir64_gen gen-obj.o
gen-and-spir64-obj.o -Xsycl-target-backend "-device *"
```
A fix for now is to ignore the autodetected spir64 target when
propagating the -Xsycl-target-backend arguments. A permanent solution
would involve a re-design of -Xsycl-target-backend handling so that it
took place only once in the flow, or belating the addition of the
autodetected generic triple into the list of device targets.

Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>
Signed-off-by: Artem Gindinson <artem.gindinson@intel.com>
@AGindinson AGindinson marked this pull request as ready for review September 2, 2021 13:15
@AGindinson AGindinson requested a review from Naghasan September 2, 2021 13:15
@AGindinson
Copy link
Contributor Author

Will investigate the Linux failures; I do observe the same crashes locally.

@AGindinson
Copy link
Contributor Author

AGindinson commented Sep 3, 2021

TODO:

  • Rework the regression test. At the moment, reverting the driver library changes doesn't lead to LIT test failure (although does lead to the expected failure in the full reproducer).

UPD: My mistake, re-building the older version correctly introduces the regression back, so nothing to do here.

@AGindinson AGindinson requested a review from mdtoguchi September 3, 2021 09:21
Copy link
Contributor

@hchilama hchilama left a comment

Choose a reason for hiding this comment

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

LGTM

@romanovvlad romanovvlad merged commit f5c838c into intel:sycl Sep 6, 2021
@AGindinson AGindinson deleted the target-backend-triple branch September 6, 2021 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants