Skip to content

[FMV][AArch64] Fix UPASS when "SSBS not fully self-synchronizing". #167

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
Merged
merged 3 commits into from
Oct 1, 2024
Merged

[FMV][AArch64] Fix UPASS when "SSBS not fully self-synchronizing". #167

merged 3 commits into from
Oct 1, 2024

Conversation

labrinea
Copy link
Contributor

@labrinea labrinea commented Sep 30, 2024 •
edited
Loading

When running try_ssbs2() on hardware which is affected by the "SSBS not fully self-synchronizing" errata, the linux kernel mutes the detection of ssbs2 via hardware caps. As a result the default version ends up running the ssbs2 code which was expected to trap originally.

To work around this UPASS I am passing an additional macro parameter to indicate whether the default version is exempt from diagnostics.

This fixes llvm/llvm-project#109304.

@labrinea
[FMV][AArch64] Fix UPASS when "SSBS not fully self-synchronizing".
d2a74ce 
When running try_ssbs2() on hardware which is affected by the "SSBS
not fully self-synchronizing" errata, the linux kernel mutes the
detection of ssbs2 via hardware caps. As a result the default version
ends up running the ssbs2 code which was expected to trap originally.

To work around this UPASS I have created a whitelist of functions
which are expected to return a different status result.

This fixes llvm/llvm-project#109304.
nikic
nikic reviewed Sep 30, 2024
View reviewed changes
jroelofs
jroelofs reviewed Sep 30, 2024
View reviewed changes
jroelofs
jroelofs reviewed Sep 30, 2024
View reviewed changes
@labrinea
Changes from last revision:
122b97c 
* replaced the exemption list with an additional macro parameter
* added comments explaining the UPASS for ssbs2
* added surrounding parentheses for the exit condition
@labrinea labrinea requested review from nikic and jroelofs September 30, 2024 16:01
nikic
nikic reviewed Sep 30, 2024
View reviewed changes
@labrinea
Changes from last revision:
80eefce 
Don't stringify IS_EXEMPT macro parameter.
nikic
nikic approved these changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@labrinea labrinea merged commit 4403710 into llvm:main Oct 1, 2024
1 check passed
@labrinea labrinea deleted the fix-ssbs branch October 1, 2024 09:15
@nikic nikic mentioned this pull request Oct 1, 2024
Closed
@pawosm-arm
Copy link

pawosm-arm commented May 20, 2025 •
edited
Loading

@labrinea I'm seeing UPASS on more features when running this test on neoverse-v1 and neoverse-v2 running RHEL8 in a docker image:

  • on neoverse-v1 these are: dpb2, bf16, i8mm, rng (/proc/cpuinfo features: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm ssbs paca pacg dcpodp svei8mm svebf16 i8mm bf16 dgh rng)

  • on neoverse-v2 these are: dpb2, bf16, i8mm, rng, bti, frintts, sve2, sve2-aes, sve2-bitperm, sve2-sha3 (/proc/cpuinfo features: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm ssbs sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti)

Of course changing the parameter value from false to true for each of these features make this test pass, but I suppose it may not be the right thing to do.

@labrinea
Copy link
Contributor Author

@pawosm-arm without looking at any logs I can only guess what's going on. We have a builder in place with similar hardware to yours https://lab.llvm.org/buildbot/#/builders/198 which makes me think the problem may relate to your operating system or the command line used. This issue may relate to yours llvm/llvm-project#123383.

@pawosm-arm
Copy link

@pawosm-arm without looking at any logs I can only guess what's going on.

There are no logs. I don't need to run entire suite for this to reproduce it:

$ clang -DNDEBUG  -O3 -fmath-errno    -O3 -DNDEBUG --rtlib=compiler-rt -o acle-fmv-features acle-fmv-features.c

$ ./acle-fmv-features
flagm
flagm2
        UPASS
dotprod
sha3
rdm
lse
sha2
aes
rcpc
rcpc2
fcma
jscvt
dpb
dpb2
        UPASS
bf16
        UPASS
i8mm
        UPASS
dit
fp16
ssbs
bti
        UPASS
simd
fp
crc
sme
sme2
f32mm
f64mm
fp16fml
frintts
        UPASS
rcpc3
rng
        UPASS
sve
sve2
        UPASS
sve2-aes
        UPASS
sve2-bitperm
        UPASS
sve2-sha3
        UPASS
sve2-sm4
wfxt
sb
sm4
sme-f64f64
sme-i16i64
mops
memtag
cssc

@labrinea
Copy link
Contributor Author

What happens if you specify -mcpu=neoverse-v2 (or v1 depending on which machine you're running this on)? Any better?

@pawosm-arm
Copy link

pawosm-arm commented May 23, 2025 •
edited
Loading

What happens if you specify -mcpu=neoverse-v2 (or v1 depending on which machine you're running this on)? Any better?

Tried this on neoverse-v2. Doesn't seem to be any different:

$ clang -DNDEBUG  -O3 -fmath-errno    -O3 -DNDEBUG --rtlib=compiler-rt -o acle-fmv-features acle-fmv-features.c -mcpu=neoverse-v2
clang: warning: math errno enabled by '-fmath-errno' after it was implicitly disabled by '-fveclib=ArmPL', this may limit the utilization of the vector library [-Wmath-errno-enabled-with-veclib]
acle-fmv-features.c:195:55: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
  195 |         asm volatile("mrs %0, ID_AA64PFR1_EL1" : "=r"(val));
      |                                                       ^
acle-fmv-features.c:195:27: note: use constraint modifier "w"
  195 |         asm volatile("mrs %0, ID_AA64PFR1_EL1" : "=r"(val));
      |                           ^~
      |                           %w0
acle-fmv-features.c:22:13: note: expanded from macro 'CHECK'
   22 |             BODY \
      |             ^
acle-fmv-features.c:382:13: warning: omitting the parameter name in a function definition is a C23 extension [-Wc23-extensions]
  382 | int main(int, const char **) {
      |             ^
acle-fmv-features.c:382:28: warning: omitting the parameter name in a function definition is a C23 extension [-Wc23-extensions]
  382 | int main(int, const char **) {
      |                            ^
3 warnings generated.

$ ./acle-fmv-features
flagm
flagm2
	UPASS
dotprod
sha3
rdm
lse
sha2
aes
rcpc
rcpc2
fcma
jscvt
dpb
dpb2
	UPASS
bf16
i8mm
dit
fp16
ssbs
bti
simd
fp
crc
sme
sme2
f32mm
f64mm
fp16fml
frintts
	UPASS
rcpc3
rng
sve
sve2
sve2-aes
	UPASS
sve2-bitperm
	UPASS
sve2-sha3
	UPASS
sve2-sm4
wfxt
sb
sm4
sme-f64f64
sme-i16i64
mops
memtag
cssc

@pawosm-arm
Copy link

pawosm-arm commented May 23, 2025 •
edited
Loading

Happens also at -O1:

$ clang -DNDEBUG  -O1 -DNDEBUG --rtlib=compiler-rt -o acle-fmv-features acle-fmv-features.c
acle-fmv-features.c:195:55: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths]
  195 |         asm volatile("mrs %0, ID_AA64PFR1_EL1" : "=r"(val));
      |                                                       ^
acle-fmv-features.c:195:27: note: use constraint modifier "w"
  195 |         asm volatile("mrs %0, ID_AA64PFR1_EL1" : "=r"(val));
      |                           ^~
      |                           %w0
acle-fmv-features.c:22:13: note: expanded from macro 'CHECK'
   22 |             BODY \
      |             ^
acle-fmv-features.c:382:13: warning: omitting the parameter name in a function definition is a C23 extension [-Wc23-extensions]
  382 | int main(int, const char **) {
      |             ^
acle-fmv-features.c:382:28: warning: omitting the parameter name in a function definition is a C23 extension [-Wc23-extensions]
  382 | int main(int, const char **) {
      |                            ^
3 warnings generated.

$ ./acle-fmv-features
flagm
flagm2
        UPASS
dotprod
sha3
rdm
lse
sha2
aes
rcpc
rcpc2
fcma
jscvt
dpb
dpb2
        UPASS
bf16
        UPASS
i8mm
        UPASS
dit
fp16
ssbs
bti
        UPASS
simd
fp
crc
sme
sme2
f32mm
f64mm
fp16fml
frintts
        UPASS
rcpc3
rng
        UPASS
sve
sve2
        UPASS
sve2-aes
        UPASS
sve2-bitperm
        UPASS
sve2-sha3
        UPASS
sve2-sm4
wfxt
sb
sm4
sme-f64f64
sme-i16i64
mops
memtag
cssc

@pawosm-arm
Copy link

...and at -O0 too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikic nikic nikic approved these changes

@tmatheson-arm tmatheson-arm Awaiting requested review from tmatheson-arm

@jroelofs jroelofs Awaiting requested review from jroelofs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

acle-fmv-features.c fails on Fedora
4 participants
@labrinea @pawosm-arm @nikic @jroelofs