Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Do not use AVX2 instructions if the CPU doesn't support it #67

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Do not use AVX2 instructions if the CPU doesn't support it #67

wants to merge 2 commits into from

Conversation

edwintorok
Copy link

The code for checking CPUID and alternate non-avx2 implementations were all mostly in place, except:

  • xgetbv was used before checking CPUID, which caused a SIGILL
  • the AVX2 codepath was used due to incorrect use of the PREAVX2_MASK flag (it always picked the avx2 path, even on non-avx2 cpus)

Fixes #9

This code is part of the Phoronix test suite, and I noticed that it crashed on AMD Opteron Processor 6172, now it no longer crashes:

./SVT-VP9/Bin/Release/SvtVp9EncApp -i Bosphorus_1920x1080_120fps_420_8bit_YUV.yuv -w 1920 -h 1080
-------------------------------------
SVT-VP9 Encoder v0.1.0
SVT [version]   : SVT-VP9 Encoder Lib v0.1.0
SVT [build]     : GCC 6.3.0      64 bit
LIB Build date: Oct 16 2019 15:36:57
-------------------------------------------
Number of logical cores available: 24
Number of PPCS 75
------------------------------------------- 
SVT [config]: Profile [0]       Level (auto)    
SVT [config]: EncoderMode / Tune                                                : 9 / 1 
SVT [config]: EncoderBitDepth                                                   : 8 
SVT [config]: SourceWidth / SourceHeight                                        : 1920 / 1080 
SVT [config]: FrameRate / Gop Size                                              : 60 / 64 
SVT [config]: HierarchicalLevels / BaseLayerSwitchMode / PredStructure          : 4 / 0 / 2 
SVT [config]: BRC Mode / QP                                                     : CQP / 45 
------------------------------------------- 


Encoding       600
SUMMARY --------------------------------- Channel 1  --------------------------------
Total Frames            Frame Rate              Byte Count              Bitrate
         600            60.00 fps                  3178622              2542.90 kbps


Channel 1
Average Speed:          8.90 fps
Total Encoding Time:    67391 ms
Total Execution Time:   68054 ms
Average Latency:        12944 ms
Max Latency:            17889 ms
Encoder finished

Note: other SVT projects suffer from same problem:

Oct 10 09:52:32 localhost kernel: [21674.538922] traps: SvtAv1EncApp[71948] trap invalid opcode ip:7f53d9187db1 sp:7f531a25c2a8 error:0
Oct 10 09:52:32 localhost kernel: [21674.540087]  in libSvtAv1Enc.so.0.7.0[7f53d8f35000+426000]
Oct 10 09:52:38 localhost kernel: [21680.858543] traps: SvtAv1EncApp[71970] trap invalid opcode ip:7f9fecf89db1 sp:7f9f2e05e2a8 error:0
Oct 10 09:52:38 localhost kernel: [21680.859492]  in libSvtAv1Enc.so.0.7.0[7f9fecd37000+426000]
Oct 10 09:52:45 localhost kernel: [21687.190615] traps: SvtAv1EncApp[72002] trap invalid opcode ip:7f06fb014db1 sp:7f063c0e92a8 error:0
Oct 10 09:52:45 localhost kernel: [21687.191559]  in libSvtAv1Enc.so.0.7.0[7f06fadc2000+426000]
Oct 10 09:52:56 localhost kernel: [21698.790049] traps: SvtAv1EncApp[72024] trap invalid opcode ip:7f960023adb1 sp:7f953289e2a8 error:0
Oct 10 09:52:56 localhost kernel: [21698.790995]  in libSvtAv1Enc.so.0.7.0[7f95fffe8000+426000]
Oct 10 09:53:02 localhost kernel: [21704.346125] traps: SvtAv1EncApp[72046] trap invalid opcode ip:7faf63669db1 sp:7fae95ccd2a8 error:0
Oct 10 09:53:02 localhost kernel: [21704.347072]  in libSvtAv1Enc.so.0.7.0[7faf63417000+426000]
Oct 10 09:53:07 localhost kernel: [21709.915184] traps: SvtAv1EncApp[72068] trap invalid opcode ip:7fe10dc54db1 sp:7fe0402b82a8 error:0
Oct 10 09:53:07 localhost kernel: [21709.916143]  in libSvtAv1Enc.so.0.7.0[7fe10da02000+426000]
Oct 10 09:53:19 localhost kernel: [21721.238068] traps: SvtAv1EncApp[72090] trap invalid opcode ip:7f73bec54db1 sp:7f72f12b82a8 error:0
Oct 10 09:53:19 localhost kernel: [21721.239006]  in libSvtAv1Enc.so.0.7.0[7f73bea02000+426000]
Oct 10 09:53:24 localhost kernel: [21726.535248] traps: SvtAv1EncApp[72112] trap invalid opcode ip:7fdf8bee5db1 sp:7fdebe5492a8 error:0
Oct 10 09:53:24 localhost kernel: [21726.536234]  in libSvtAv1Enc.so.0.7.0[7fdf8bc93000+426000]
Oct 10 09:53:29 localhost kernel: [21731.826031] traps: SvtAv1EncApp[72134] trap invalid opcode ip:7fd42dfd3db1 sp:7fd3606372a8 error:0
Oct 10 09:53:29 localhost kernel: [21731.826972]  in libSvtAv1Enc.so.0.7.0[7fd42dd81000+426000]
Oct 10 09:53:40 localhost kernel: [21742.131875] traps: SvtHevcEncApp[72152] trap invalid opcode ip:7f403535e8e2 sp:7ffe823fee18 error:0
Oct 10 09:53:40 localhost kernel: [21742.132861]  in libSvtHevcEnc.so.1[7f4035343000+382000]
Oct 10 09:53:44 localhost kernel: [21746.240475] traps: SvtHevcEncApp[72165] trap invalid opcode ip:7fc93648d8e2 sp:7ffd20bc3f58 error:0
Oct 10 09:53:44 localhost kernel: [21746.241422]  in libSvtHevcEnc.so.1[7fc936472000+382000]
Oct 10 09:53:48 localhost kernel: [21750.352681] traps: SvtHevcEncApp[72168] trap invalid opcode ip:7f0e777f48e2 sp:7fff06b84988 error:0
Oct 10 09:53:48 localhost kernel: [21750.353631]  in libSvtHevcEnc.so.1[7f0e777d9000+382000]
Oct 10 09:53:58 localhost kernel: [21760.547172] traps: SvtVp9EncApp[72171] trap invalid opcode ip:7f05a7948328 sp:7fffea3f6840 error:0
Oct 10 09:53:58 localhost kernel: [21760.548142]  in libSvtVp9Enc.so.1[7f05a793a000+106000]
Oct 10 09:54:02 localhost kernel: [21764.660897] traps: SvtVp9EncApp[72174] trap invalid opcode ip:7f4cc37ab328 sp:7ffecdb04ee0 error:0
Oct 10 09:54:02 localhost kernel: [21764.661837]  in libSvtVp9Enc.so.1[7f4cc379d000+106000]
Oct 10 09:54:06 localhost kernel: [21768.777223] traps: SvtVp9EncApp[72177] trap invalid opcode ip:7f0c44e87328 sp:7fff6b198c00 error:0
Oct 10 09:54:06 localhost kernel: [21768.778171]  in libSvtVp9Enc.so.1[7f0c44e79000+106000]

@edwintorok
First check CPUID, and then use xgetbv
eb2f70b 
Otherwise we crash with SIGILL on a CPU without AVX (e.g. AMD Opteron 6172).

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
@edwintorok
Use non-AVX2 codepaths on non-AVX2 CPUs
0dc9806 
To avoid a SIGILL on non-AVX2 CPU use the correct codepath.
`ASM_TYPES & PREAVX2_MASK` is always `1`, which chooses the avx2 path
and crashes.
Use `ASM_TYPES & AVX2_MASK` which should be `0` if the CPU doesn't
support AVX2, choosing the non-AVX2 path and avoiding a SIGILL.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
@edwintorok
Copy link
Author

Is there a test suite I could run to check whether it also works correctly?

@tianjunwork
Copy link
Contributor

Hi @edwintorok, thank you for this PR.
There isn't a full test suite for VP9. We can fork one from HEVC and change from there. I will keep you updated.

@edwintorok
Copy link
Author

The appveyor CI error seems unrelated:

Maximum allowed artifact storage size of 50000 Mb will be exceeded.

@tianjunwork
Copy link
Contributor

Hi @edwintorok, yes, it is not related.

@tianjunwork
Copy link
Contributor

Hi @edwintorok, fyi, before merging this PR, there is some effort to evaluate performance on legacy CPU. With the scarcity of legacy HW, it will take longer for us to move forward with this PR.

@eisneinechse eisneinechse mentioned this pull request Jul 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Don't use AVX2 instructions on unsupported cpus
2 participants
@edwintorok @tianjunwork