[v2] gdb/testsuite: make gdb.reverse/i386-avx-reverse.exp require also avx2
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Test passed
|
Commit Message
I just realized the patch did not work properly, hence this v2. as
I was calling cpuid incorrectly - one has to also set ecx to 0:
- if (!x86_cpuid (7, &eax, &ebx, &ecx, &edx))
+ if (!x86_cpuid_count (7, 0, &eax, &ebx, &ecx, &edx))
I must have been looking at wrong log/terminal when testing.
Below is the fixed version I hope. Sorry for the noise.
Jan
-- >8 --
Subject: [PATCH v2] gdb/testsuite: make gdb.reverse/i386-avx-reverse.exp
The test gdb.reverse/i386-avx-reverse.exp requires CPU to have AVX
instructions but it actually also uses AVX2 instructions (like
vpcmpeqd). This caused the test to fail on CPUs that have AVX but not
AVX2.
This commit adds check for AVX2.
Tested on Intel Xeon CPU E3-1265L (no AVX2) and Intel Core i7-1355U
(has AVX2).
---
.../gdb.reverse/i386-avx-reverse.exp | 1 +
gdb/testsuite/lib/gdb.exp | 45 +++++++++++++++++++
2 files changed, 46 insertions(+)
Comments
Polite ping.
On Wed, 2024-12-18 at 11:18 +0000, Jan Vrany wrote:
> I just realized the patch did not work properly, hence this v2. as
> I was calling cpuid incorrectly - one has to also set ecx to 0:
>
> - if (!x86_cpuid (7, &eax, &ebx, &ecx, &edx))
> + if (!x86_cpuid_count (7, 0, &eax, &ebx, &ecx, &edx))
>
> I must have been looking at wrong log/terminal when testing.
> Below is the fixed version I hope. Sorry for the noise.
>
> Jan
>
> -- >8 --
> Subject: [PATCH v2] gdb/testsuite: make gdb.reverse/i386-avx-
> reverse.exp
>
> The test gdb.reverse/i386-avx-reverse.exp requires CPU to have AVX
> instructions but it actually also uses AVX2 instructions (like
> vpcmpeqd). This caused the test to fail on CPUs that have AVX but not
> AVX2.
>
> This commit adds check for AVX2.
>
> Tested on Intel Xeon CPU E3-1265L (no AVX2) and Intel Core i7-1355U
> (has AVX2).
> ---
> .../gdb.reverse/i386-avx-reverse.exp | 1 +
> gdb/testsuite/lib/gdb.exp | 45
> +++++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> index cc920d3a2f3..6fe099f6942 100644
> --- a/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> +++ b/gdb/testsuite/gdb.reverse/i386-avx-reverse.exp
> @@ -22,6 +22,7 @@
>
> require supports_reverse
> require have_avx
> +require have_avx2
>
> # TODO: this is the case because I used xmm15 all over the test.
> # Some parts of the test require xmm15 to validate some code paths,
> but
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 7ee2043f0f8..eb94124f04b 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -10427,6 +10427,51 @@ gdb_caching_proc have_avx {} {
> return $status
> }
>
> +# Return 1 if target supports avx2, otherwise return 0.
> +gdb_caching_proc have_avx2 {} {
> + global srcdir
> +
> + set me "have_avx2"
> + if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } {
> + verbose "$me: target does not support avx2, returning 0"
> 2
> + return 0
> + }
> +
> + # Compile a test program.
> + set src {
> + #include "nat/x86-cpuid.h"
> +
> + int main() {
> + unsigned int eax, ebx, ecx, edx;
> +
> + if (!x86_cpuid_count (7, 0, &eax, &ebx, &ecx, &edx))
> + return 0;
> +
> + if ((ebx & bit_AVX2) == bit_AVX2)
> + return 1;
> + else
> + return 0;
> + }
> + }
> + set compile_flags "incdir=${srcdir}/.."
> + if {![gdb_simple_compile $me $src executable
> $compile_flags]} {
> + return 0
> + }
> +
> + set target_obj [gdb_remote_download target $obj]
> + set result [remote_exec target $target_obj]
> + set status [lindex $result 0]
> + set output [lindex $result 1]
> + if { $output != "" } {
> + set status 0
> + }
> +
> + remote_file build delete $obj
> +
> + verbose "$me: returning $status" 2
> + return $status
> +}
> +
> # Called as
> # - require ARG...
> #
>>>>> "Jan" == Jan Vrany <jan.vrany@labware.com> writes:
Jan> I just realized the patch did not work properly, hence this v2. as
Jan> I was calling cpuid incorrectly - one has to also set ecx to 0:
Jan> - if (!x86_cpuid (7, &eax, &ebx, &ecx, &edx))
Jan> + if (!x86_cpuid_count (7, 0, &eax, &ebx, &ecx, &edx))
Jan> I must have been looking at wrong log/terminal when testing.
Jan> Below is the fixed version I hope. Sorry for the noise.
Ok, thank you.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
@@ -22,6 +22,7 @@
require supports_reverse
require have_avx
+require have_avx2
# TODO: this is the case because I used xmm15 all over the test.
# Some parts of the test require xmm15 to validate some code paths, but
@@ -10427,6 +10427,51 @@ gdb_caching_proc have_avx {} {
return $status
}
+# Return 1 if target supports avx2, otherwise return 0.
+gdb_caching_proc have_avx2 {} {
+ global srcdir
+
+ set me "have_avx2"
+ if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } {
+ verbose "$me: target does not support avx2, returning 0" 2
+ return 0
+ }
+
+ # Compile a test program.
+ set src {
+ #include "nat/x86-cpuid.h"
+
+ int main() {
+ unsigned int eax, ebx, ecx, edx;
+
+ if (!x86_cpuid_count (7, 0, &eax, &ebx, &ecx, &edx))
+ return 0;
+
+ if ((ebx & bit_AVX2) == bit_AVX2)
+ return 1;
+ else
+ return 0;
+ }
+ }
+ set compile_flags "incdir=${srcdir}/.."
+ if {![gdb_simple_compile $me $src executable $compile_flags]} {
+ return 0
+ }
+
+ set target_obj [gdb_remote_download target $obj]
+ set result [remote_exec target $target_obj]
+ set status [lindex $result 0]
+ set output [lindex $result 1]
+ if { $output != "" } {
+ set status 0
+ }
+
+ remote_file build delete $obj
+
+ verbose "$me: returning $status" 2
+ return $status
+}
+
# Called as
# - require ARG...
#