Issue 289624514: 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480
status:  New
reporter:  as...@gmail.com
cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug
priority:  P4
severity:  S2
retention:  Component default


as...@gmail.com added comment #1:
cmake -DCMAKE_INSTALL_PREFIX="$installdir" -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_STANDARD=17 -DLIBGAV1_ENABLE_TESTS=ON -DBUILD_SHARED_LIBS=ON ..
  for d in $(ls -d CMakeFiles/*.dir); do
    [[ -f "$d"/link.txt ]] && sed -i 's/\(-lpthread[ ]*\)$/-lregex \1/' "$d"/link.txt
  done

  make
  ctest

4/59 Test  #4: common_avx2_test ..................***Exception: Illegal  0.40 sec
alexei@Aspire:~/build/libgav1/mybuild$ ./common_avx2_test
Running main() from /home/alexei/build/libgav1/third_party/googletest/googletest/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from CommonDspTest
[ RUN      ] CommonDspTest.AVX2RightShiftWithRoundingS16
Недопустимая инструкция (стек памяти сброшен на диск)




my_system.txt
ViewDownload

Generated by Google IssueTracker notification system

You're receiving this email because you are subscribed to updates on Google IssueTracker issue 289624514. 
Unsubscribe from this issue.

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed

as...@gmail.com added comment #2:
At CmakeLists.txt I set
set(CMAKE_CXX_STANDARD 17)
set(ABSL_CXX_STANDARD 17)


_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480
status:  New
reporter:  as...@gmail.com
cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug
priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed

jz...@google.com added comment #3:

Thanks for the report. The issue is that your processor, the Intel(R) Pentium(R) CPU N3700, doesn't support AVX2 instructions and the test is running without checking. The main library should detect this at runtime and not cause a SIGILL.

For now you can add -E common_avx2_test to the ctest command line. You'll likely need to add more of the dsp tests too. There's not a great way to forward test arguments (--gtest_filter=-*AVX2* for example) to the tests through ctest.

I'll add a check in this test and take a look at the others. Something similar to the following should work:

diff --git a/src/dsp/x86/common_avx2_test.cc b/src/dsp/x86/common_avx2_test.cc
index 2062683..11c74b1 100644
--- a/src/dsp/x86/common_avx2_test.cc
+++ b/src/dsp/x86/common_avx2_test.cc
@@ -21,6 +21,7 @@
 #include <cstdint>
 
 #include "src/utils/common.h"
+#include "src/utils/cpu.h"
 
 namespace libgav1 {
 namespace dsp {
@@ -32,6 +33,7 @@ namespace {
 // RightShiftWithRounding_S16() is equal to RightShiftWithRounding() only for
 // negative values.
 TEST(CommonDspTest, AVX2RightShiftWithRoundingS16) {
+  if ((GetCpuInfo() & kAVX2) == 0) GTEST_SKIP() << "No AVX2 support";
   for (int bits = 0; bits < 16; ++bits) {
     const int bias = (1 << bits) >> 1;
     for (int32_t value = INT16_MIN; value <= INT16_MAX; ++value) {
diff --git a/tests/libgav1_tests.cmake b/tests/libgav1_tests.cmake
index 9c3fdb7..6428020 100644
--- a/tests/libgav1_tests.cmake
+++ b/tests/libgav1_tests.cmake
@@ -284,6 +284,8 @@ macro(libgav1_add_tests_targets)
                            ${libgav1_defines}
                            INCLUDES
                            ${libgav1_test_include_paths}
+                           OBJLIB_DEPS
+                           libgav1_utils
                            LIB_DEPS
                            ${libgav1_common_test_absl_deps}
                            libgav1_gtest

_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480
status:  New
reporter:  as...@gmail.com
cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug
priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed

jz...@google.com added comment #4:

Looking a little more closely, the test will need to be split to a fixture class that isn't built with AVX2 to allow the check to run without the risk of AVX/AVX2 instructions being used when registering the test.

For now, ctest -E common_avx2_test should work.

_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480
status:  New
reporter:  as...@gmail.com
cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug
priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed

as...@gmail.com added comment #5:
thank you!


06.07.2023 01:54, buganize...@google.com пишет:

> Replying to this email means your email address will be shared with
> the team that works on this product.
>
> https://issuetracker.google.com/issues/289624514
>

> *Changed*
>
> *jz...@google.com added comment #3
> <https://issuetracker.google.com/issues/289624514#comment3>:*

>
> Thanks for the report. The issue is that your processor, the Intel(R)
> Pentium(R) CPU N3700, doesn't support AVX2 instructions and the test
> is running without checking. The main library should detect this at
> runtime and not cause a SIGILL.
>
> For now you can add |-E common_avx2_test| to the |ctest| command line.
> You'll likely need to add more of the dsp tests too. There's not a

> great way to forward test arguments (|--gtest_filter=-*AVX2*| for

> example) to the tests through |ctest|.
>
> I'll add a check in this test and take a look at the others. Something
> similar to the following should work:
>
> |diff --git a/src/dsp/x86/common_avx2_test.cc
> b/src/dsp/x86/common_avx2_test.cc index 2062683..11c74b1 100644 ---
> a/src/dsp/x86/common_avx2_test.cc +++
> b/src/dsp/x86/common_avx2_test.cc @@ -21,6 +21,7 @@ #include <cstdint>
> #include "src/utils/common.h" +#include "src/utils/cpu.h" namespace
> libgav1 { namespace dsp { @@ -32,6 +33,7 @@ namespace { //
> RightShiftWithRounding_S16() is equal to RightShiftWithRounding() only
> for // negative values. TEST(CommonDspTest,
> AVX2RightShiftWithRoundingS16) { + if ((GetCpuInfo() & kAVX2) == 0)
> GTEST_SKIP() << "No AVX2 support"; for (int bits = 0; bits < 16;
> ++bits) { const int bias = (1 << bits) >> 1; for (int32_t value =
> INT16_MIN; value <= INT16_MAX; ++value) { diff --git
> a/tests/libgav1_tests.cmake b/tests/libgav1_tests.cmake index
> 9c3fdb7..6428020 100644 --- a/tests/libgav1_tests.cmake +++
> b/tests/libgav1_tests.cmake @@ -284,6 +284,8 @@
> macro(libgav1_add_tests_targets) ${libgav1_defines} INCLUDES
> ${libgav1_test_include_paths} + OBJLIB_DEPS + libgav1_utils LIB_DEPS
> ${libgav1_common_test_absl_deps} libgav1_gtest |
>
>
> _______________________________
>

> *Reference Info: 289624514 4/59 Test #4: common_avx2_test
> ..................***Exception: Illegal 0.40 sec*
> component: Public Trackers > libgav1
> <https://issuetracker.google.com/components/750480>
> status: New
> reporter: asholom...@gmail.com
> cc: asholom...@gmail.com, ga...@googlegroups.com

> type: Bug
> priority: P4
> severity: S2
> retention: Component default
>
>
>

> Generated by Google IssueTracker notification system
>
> You're receiving this email because you are subscribed to updates on
> Google IssueTracker issue 289624514

> <https://issuetracker.google.com/issues/289624514> where you have the
> roles: reporter, cc.
> Unsubscribe from this issue.
> <https://issuetracker.google.com/issues/289624514?unsubscribe=true>

_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480
status:  New
reporter:  as...@gmail.com
cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug
priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed
status:  New  →  Assigned
assignee:  <none>  →  jz...@google.com

_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480

status:  Assigned
reporter:  as...@gmail.com
assignee:  jz...@google.com

cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug

access level:  Default access

priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed

ap...@google.com added comment #6:
Project: codecs/libgav1
Branch: main

commit 2bb1ffb4bedf56598ff8c628b20983aa1531dd35
Author: James Zern <jz...@google.com>
Date:   Wed Jul 19 18:54:55 2023

    dsp/*_test.cc: check AVX2/SSE4 support before testing
   
    This avoids generating SIGILLs on machines without support for these
    instructions. The tests are now skipped rather than just skipping the
    *Init() calls which would result in misleading output as the base C++
    code would be used.
   
    Tested with Intel Software Development Emulator (SDE) using Silvermont
    (no AVX2) and Pentium 4 Prescott (no SSE4). common_avx2_test and
    common_sse4_test will be addressed in a follow up.
   
    Bug: b/289624514
    PiperOrigin-RevId: 549488461
    Change-Id: I89585122b53ea114f0e02baa594ac0a430bc7a9a

M       src/dsp/average_blend_test.cc
M       src/dsp/cdef_test.cc
M       src/dsp/convolve_test.cc
M       src/dsp/distance_weighted_blend_test.cc
M       src/dsp/intra_edge_test.cc
M       src/dsp/intrapred_cfl_test.cc
M       src/dsp/intrapred_directional_test.cc
M       src/dsp/intrapred_filter_test.cc
M       src/dsp/intrapred_test.cc
M       src/dsp/inverse_transform_test.cc
M       src/dsp/loop_filter_test.cc
M       src/dsp/loop_restoration_test.cc
M       src/dsp/mask_blend_test.cc
M       src/dsp/motion_field_projection_test.cc
M       src/dsp/motion_vector_search_test.cc
M       src/dsp/obmc_test.cc
M       src/dsp/super_res_test.cc
M       src/dsp/warp_test.cc
M       src/dsp/weight_mask_test.cc
M       src/film_grain_test.cc
M       src/reconstruction_test.cc

https://chromium-review.googlesource.com/4706716


_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480

status:  Assigned
reporter:  as...@gmail.com
assignee:  jz...@google.com

cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug

access level:  Default access

priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed

ap...@google.com added comment #7:
Project: codecs/libgav1
Branch: main

commit 0fc6d78566a38d0b58d61a104dee5322d23adc23
Author: James Zern <jz...@google.com>
Date:   Mon Jul 24 16:26:40 2023

    add common_dsp_test.cc
   
    Use this to call AVX2/SSE4 specific functions, formerly in the
    common_{avx2,sse4}_test tests. This avoids invoking the tests without
    checking for CPU support.
   
    The test fixture needs to be in a separate file to avoid AVX2/SSE4
    instructions being generated in the fixture or gtest support code.
    common_neon_test.cc is left as is since NEON is a requirement for Arm
    builds currently.
   
    Bug: b/289624514
    PiperOrigin-RevId: 550705795
    Change-Id: Idd984e3f94d0c5b1e9d1bbc8d2fc376e60c17cc5

A       src/dsp/common_dsp_test.cc
M       src/dsp/x86/common_avx2_test.cc
A       src/dsp/x86/common_avx2_test.h
M       src/dsp/x86/common_sse4_test.cc
A       src/dsp/x86/common_sse4_test.h
M       tests/libgav1_tests.cmake

https://chromium-review.googlesource.com/4711424


_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480

status:  Assigned
reporter:  as...@gmail.com
assignee:  jz...@google.com

cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug

access level:  Default access

priority:  P4
severity:  S2
retention:  Component default

[buganize...@google.com]

Replying to this email means your email address will be shared with the team that works on this product.

https://issuetracker.google.com/issues/289624514

Changed
status:  Assigned  →  Fixed

jz...@google.com added comment #8:

Thanks again for the report. After the change in comment #7 you should be able to run all the tests without any workarounds. Tests will be skipped if the device does not support the instructions.

_______________________________

Reference Info: 289624514 4/59 Test #4: common_avx2_test ..................***Exception: Illegal 0.40 sec

component:  750480

status:  Fixed
reporter:  as...@gmail.com
assignee:  jz...@google.com

cc:  as...@gmail.com, ga...@googlegroups.com
type:  Bug

access level:  Default access

priority:  P4
severity:  S2
retention:  Component default