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

[SYCL][CUDA] Unoptimized stream regression test #362

Merged
merged 2 commits into from
Jul 21, 2021

Conversation

steffenlarsen
Copy link

Previously the CUDA backend would fail due to invalid atomic memory orders not being optimized out. The use of these invalid memory orders have been removed in intel/llvm#4106, so this commit adds a regression test to make sure they do not resurface.

Previously the CUDA backend would fail due to invalid atomic memory
orders not being optimized out. The use of these invalid memory orders
have been removed in recent changes, so this commit adds a regression
test to make sure they do not resurface.

Signed-off-by: Steffen Larsen <[email protected]>
@vladimirlaz vladimirlaz requested a review from bader July 14, 2021 13:00
bader
bader previously approved these changes Jul 14, 2021
Copy link

@bader bader left a comment

Choose a reason for hiding this comment

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

Previously the CUDA backend would fail due to invalid atomic memory orders not being optimized out. The use of these invalid memory orders have been removed in intel/llvm#4106, so this commit adds a regression test to make sure they do not resurface.

The test look good to me.
I would add the motivation to have this regression test to code comments instead of (or in addition to) the commit message.

@steffenlarsen
Copy link
Author

I would add the motivation to have this regression test to code comments instead of (or in addition to) the commit message.

Good call! I'll add that once all the tests finish. Can you confirm that the only failing test in Jenkins/pre-ci-cuda is the new test? If so it should work when intel/llvm#4106 is merged.

@bader
Copy link

bader commented Jul 14, 2021

Can you confirm that the only failing test in Jenkins/pre-ci-cuda is the new test?

Yes. Only new test fails with

fatal error: error in backend: Cannot select: t3: i32,ch = AtomicLoad<(volatile load seq_cst (s32) from %ir.0, addrspace 1)> t0, t2
   t2: i64,ch = CopyFromReg t0, Register:i64 %0
     t1: i64 = Register %0
 In function: _ZNK2cl4sycl6atomicIjLNS0_6access13address_spaceE1EE4loadIjEENSt9enable_ifIXntsr3std7is_sameIfT_EE5valueEjE4typeENS0_12memory_orderE

If so it should work when intel/llvm#4106 is merged.

Let's re-run the test after intel/llvm#4106 is merged to confirm that.

vladimirlaz
vladimirlaz previously approved these changes Jul 16, 2021
Signed-off-by: Steffen Larsen <[email protected]>
@steffenlarsen steffenlarsen dismissed stale reviews from vladimirlaz and bader via 4437c55 July 20, 2021 09:00
@romanovvlad romanovvlad merged commit 1226e5c into intel:intel Jul 21, 2021
smaslov-intel pushed a commit to smaslov-intel/llvm-test-suite that referenced this pull request Aug 12, 2021
Previously the CUDA backend would fail due to invalid atomic memory
orders not being optimized out. The use of these invalid memory orders
have been removed in recent changes, so this commit adds a regression
test to make sure they do not resurface.

Signed-off-by: Steffen Larsen <[email protected]>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
…e#362)

Previously the CUDA backend would fail due to invalid atomic memory
orders not being optimized out. The use of these invalid memory orders
have been removed in recent changes, so this commit adds a regression
test to make sure they do not resurface.

Signed-off-by: Steffen Larsen <[email protected]>
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.

4 participants