-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[clang][x86] Add C/C++ and 32/64-bit test coverage to constexpr tests #152478
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
Conversation
@llvm/pr-subscribers-clang Author: Pedro Lobo (pedroclobo) ChangesThe previous test incorrectly assumed 32-bit integers, leading to incorrect bit counts. This change updates the test result to assume 64-bit integers. Full diff: https://github.com/llvm/llvm-project/pull/152478.diff 1 Files Affected:
diff --git a/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c b/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
index ca8f5e482cc56..f4de0f17caacf 100644
--- a/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
+++ b/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
@@ -8,7 +8,7 @@ __m512i test_mm512_popcnt_epi64(__m512i __A) {
// CHECK: @llvm.ctpop.v8i64
return _mm512_popcnt_epi64(__A);
}
-TEST_CONSTEXPR(match_v8di(_mm512_popcnt_epi64((__m512i)(__v8di){+5, -3, -10, +8, 0, -256, +256, -128}), 2, 31, 30, 1, 0, 24, 1, 25));
+TEST_CONSTEXPR(match_v8di(_mm512_popcnt_epi64((__m512i)(__v8di){+5, -3, -10, +8, 0, -256, +256, -128}), 2, 63, 62, 1, 0, 56, 1, 57));
__m512i test_mm512_mask_popcnt_epi64(__m512i __W, __mmask8 __U, __m512i __A) {
// CHECK-LABEL: @test_mm512_mask_popcnt_epi64
|
@llvm/pr-subscribers-backend-x86 Author: Pedro Lobo (pedroclobo) ChangesThe previous test incorrectly assumed 32-bit integers, leading to incorrect bit counts. This change updates the test result to assume 64-bit integers. Full diff: https://github.com/llvm/llvm-project/pull/152478.diff 1 Files Affected:
diff --git a/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c b/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
index ca8f5e482cc56..f4de0f17caacf 100644
--- a/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
+++ b/clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
@@ -8,7 +8,7 @@ __m512i test_mm512_popcnt_epi64(__m512i __A) {
// CHECK: @llvm.ctpop.v8i64
return _mm512_popcnt_epi64(__A);
}
-TEST_CONSTEXPR(match_v8di(_mm512_popcnt_epi64((__m512i)(__v8di){+5, -3, -10, +8, 0, -256, +256, -128}), 2, 31, 30, 1, 0, 24, 1, 25));
+TEST_CONSTEXPR(match_v8di(_mm512_popcnt_epi64((__m512i)(__v8di){+5, -3, -10, +8, 0, -256, +256, -128}), 2, 63, 62, 1, 0, 56, 1, 57));
__m512i test_mm512_mask_popcnt_epi64(__m512i __W, __mmask8 __U, __m512i __A) {
// CHECK-LABEL: @test_mm512_mask_popcnt_epi64
|
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.
nice catch - any idea why the test was passing?
I don't really know how to run/verify these I've been relying on my editor's LSP. It flagged the failing |
just realised its not being run as c++ - just plain c :( Please can you add additional RUN lines? |
avx512vpopcntdqvl-builtins.c needs additional RUN lines as well |
Oh, I see you added the run lines in |
Some files containing `constexpr` tests did not have a C++ run line.
1ad935a
to
5d3a3ec
Compare
_mm512_popcnt_epi64
constexpr testThere 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 - cheers
(PR title + summary needs a little work) |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/24875 Here is the relevant piece of the build log for the reference
|
Adds missing C++ run lines to test files containing
constexpr
tests.Also adds missing 32/64-bit test coverage to the following tests:
clang/test/CodeGen/X86/avx512-reduceIntrin.c
clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
clang/test/CodeGen/X86/avx512vpopcntdq-builtins.c
clang/test/CodeGen/X86/avx512vpopcntdqvl-builtins.c
Additionally, fixes a
_mm512_popcnt_epi64
constexpr
test that incorrectly assumed 32-bit integers, leading to incorrect bit counts. This change updates the test result to assume 64-bit integers.