Skip to content
This repository was archived by the owner on Mar 28, 2023. It is now read-only.

Conversation

@fineg74
Copy link

@fineg74 fineg74 commented Aug 23, 2022

Complementary compiler PR: intel/llvm#6627

@v-klochkov
Copy link

/verify with intel/llvm#6627

Copy link

@v-klochkov v-klochkov left a comment

Choose a reason for hiding this comment

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

Looks good to me. I have only few non-blocking comments.

{
sycl::range<1> range(1);
sycl::buffer<double, 1> buffer_in(&input, range);
sycl::buffer<unsigned int, 1> buffer_out(&output, range);

Choose a reason for hiding this comment

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

Not a requirement: as soon as the test is 'double_conversion', it may be nice to test 'signed int' as well.

Copy link
Author

Choose a reason for hiding this comment

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

Added some tests for basic test fir signed int

@v-klochkov v-klochkov requested a review from kbobrovs August 23, 2022 13:13
@fineg74
Copy link
Author

fineg74 commented Aug 23, 2022

/verify with intel/llvm#6627

@kbobrovs kbobrovs requested a review from v-klochkov August 25, 2022 17:13
@v-klochkov
Copy link

v-klochkov commented Aug 30, 2022

@fineg74 - please fix one more clang-format issue

v-klochkov
v-klochkov previously approved these changes Aug 30, 2022
@fineg74
Copy link
Author

fineg74 commented Aug 31, 2022

The test fails with error: LLVM ERROR: GenX execution width and GRF crossing legalization failed for: < %1 = fptoui <1 x double> %call2.i.i.i.i.esimd.i.i to <1 x i32>>: 'double' data type is not supported by this target
Not related to the test

v-klochkov
v-klochkov previously approved these changes Aug 31, 2022
@v-klochkov v-klochkov dismissed their stale review August 31, 2022 19:28

Right, the problem with double types is a known one and it needs additional checks in the header of the test like here: #1150 or here: #1190

Copy link

@v-klochkov v-klochkov left a comment

Choose a reason for hiding this comment

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

Please add 'aspect-fp64' to 'REQUIRES' section.

@fineg74
Copy link
Author

fineg74 commented Sep 9, 2022

@v-klochkov could you please merge the test

@v-klochkov v-klochkov merged commit 6d3dd9d into intel:intel Sep 10, 2022
@fineg74 fineg74 deleted the doubleConversionTest branch September 12, 2022 19:39
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
* Test correctness of type conversion fix
* Add requirement of aspect-fp64 for the test
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…#1170)

* Test correctness of type conversion fix
* Add requirement of aspect-fp64 for the test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants