-
Notifications
You must be signed in to change notification settings - Fork 788
Add negative tests for inline asm #2406
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
Add negative tests for inline asm #2406
Conversation
Signed-off-by: rbegam <[email protected]>
09b3fb6
to
8f7b5d2
Compare
Added new parameter to launchInlineASMTests to be able to get the exceptions out of it to check their content in negative tests. Updated spelling of `reqd_sub_group_size` attribute to avoid using deprecated variant of it.
// RUN: %clangxx -fsycl %s -DINLINE_ASM -o %t.out | ||
// RUN: not %t.out 2>&1 | FileCheck %s | ||
|
||
// CHECK: Build succeeded |
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 wonder why do we have tests that expected to be successfully built in negative_tests
directory?
If these tests require particular HW to catch device program build error, than I suggest to remove them as they are too fragile, IMHO
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, but I am not familiar with GPU asm. BTW, where it is specified which GPU HW to run on?
@@ -0,0 +1,43 @@ | |||
// UNSUPPORTED: cuda | |||
// REQUIRES: gpu,linux | |||
// RUN: %clangxx -fsycl %s -DINLINE_ASM -o %t.out |
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.
Why do you need this INLINE_ASM macro since you seem to always define it?
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.
It remained from positive tests, I will delete it.
Signed-off-by: Aleksander Fadeev <[email protected]>
Signed-off-by: Aleksander Fadeev <[email protected]>
9e81046
to
5afddf6
Compare
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
Signed-off-by: Aleksander Fadeev <[email protected]>
e2b6861
to
f4fbcb0
Compare
Don't merge please yet. |
Signed-off-by: Aleksander Fadeev <[email protected]>
I won't fix clang formatting on purpose. |
The PR can be merged now |
Add SDL hardening flags for MSVC
Add tests for verification of error messages for inline assembly feature.
Signed-off-by: Aleksander Fadeev [email protected]