Skip to content

[SYCL] Matching sizeof long double of SYCL device with that of host #1786

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

Merged
merged 1 commit into from
Jun 5, 2020

Conversation

schittir
Copy link
Contributor

static_assert of sizeof(long double)>8 fails to compile with -fsycl
switch because sizeof(long double) for SYCL device is 8 bytes whereas
the host is 16 bytes. This commit sets sizeof(long double) width to 16
bytes to match that of SYCL host for Linux gnu SYCLdevice x86_64 and x86

Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

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

AFAIK community does such things in a bit other way. See 8557d1a .

Copy link
Contributor

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

@Fznamznon : That one is interesting! I wonder if we should just do that for SYCL as well (modify that 'if' statement). I think this one is 'the right' way, but if community has already made that selection perhaps we need to as well.

Additionally, I think that method actually correctly handles the double-size flags, which this doesn't seem to do.

@Fznamznon
Copy link
Contributor

@Fznamznon : That one is interesting! I wonder if we should just do that for SYCL as well (modify that 'if' statement). I think this one is 'the right' way, but if community has already made that selection perhaps we need to as well.

I think we should be aligned with community code and generalize approaches between SYCL and other offloading models which already is presented in master (like OpenMP), because they solve similar problems quite often. One problem I know with this community method - it crashes if user didn't specify AUX triple, but It is easy to fix by adding check on AUX triple presence.
If the method which this PR exposes is better (I cannot say for sure, I'm not an expert in this place), I think we should generalize approach between OpenMP and SYCL and upstream it.

@schittir schittir force-pushed the Correcting_LongDoubleWidth_SYCLDevice branch 2 times, most recently from 801ca3b to 5ccac4c Compare June 4, 2020 02:57
static_assert of sizeof(long double)>8 fails to compile with -fsycl
switch on Linux x86_64 bit target because sizeof(long double) for SYCL device is
8 bytes whereas the host is 16 bytes.

This commit sets sizeof(long double) of SYCL device to match that of host
@schittir schittir force-pushed the Correcting_LongDoubleWidth_SYCLDevice branch from 5ccac4c to d89721c Compare June 4, 2020 04:12
Copy link
Contributor

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

Please stop force pushing. I've sent you an email on how to fix this.

@bader bader merged commit 87e6240 into intel:sycl Jun 5, 2020
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