[mlir][aarch64] Test failure: correctness.mlir

[banach-space]

Hello 👋🏻 !

I'm looking into MLIR integration tests and I've noticed that correctness.mlir is failing on AArch64. On closer inspection, I see that that test:

• runs some FP calculations, and then
• verifies the generated FP output matches the expected output.

I see the following discrepancies when running these tests on AArch64:

MISMATCH 1
EXPECTED: nan
GENERATED: -nan

MISMATCH 2
EXPECTED: 0
GENERATED: 5.96046e-08

I'm a bit more worried about the 2nd case, but haven't had the bandwidth to dive deeper. I'm not sure about the 1st mismatch.

Next step
I propose that for now we XFAL this test on AArch64. I don't have the spare cycles to investigate this further at the moment, but am happy to support volunteers :)

On FP integrations tests in LLVM in general
One other thing that's worth pointing out is that such integration tests would normally reside llvm-test-suite. In particular, FP tests would use fpcmp for comparing FP results so that relative and absolute tolerances can be taken into account. Perhaps I should bring this up on Discourse 🤔 .

Thanks for taking a look!
-Andrzej

CC @pifon2a @bixia1 @Lewuathe

[llvmbot]

@llvm/issue-subscribers-mlir

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

[Lewuathe]

I have found correctness.mlir passed with the latest main branch even in aarch64 architecture. (Apple M1 Max, Ventura). I'm not sure whether it's thanks to the update of underlying infra (i.e. fpcmp?)

I'm wondering if our CI infra supports aarch64 to avoid the regression later. How can we support new architecture in Harbormaster?

[banach-space]

Tested with f8a469f and it is still failing for me. Which revision have you checked? There might a difference between your M1 and my set-up 🤔 .

I'm wondering if our CI infra supports aarch64 to avoid the regression later. How can we support new architecture in Harbormaster?

Actually, there's a lot of AArch64 buildbots upstream: https://lab.llvm.org/buildbot/#/console :)

[Lewuathe]

The correctness.mlir test with CPU runner is compiled with the call of printF32 function defined in CRunnerUtil for f32 value.

 llvm.call @printF32(%15) : (f32) -> ()

CRunnerUtils seems to use fprintf function inside with %g specifier to print the value.

extern "C" void printF32(float f) { fprintf(stdout, "%g", f); }

Therefore I have run this code in several platforms as an experiment,

#include <stdio.h>

int main()
{
  const float nan = 0.0/0.0;
  fprintf(stdout, "nan = %g\n", nan);

  const float neg_nan = -nan;
  fprintf(stdout, "neg_nan = %g\n", neg_nan);

  const float small_value = 5.96046e-08;
  fprintf(stdout, "small_value = %g\n", small_value);
}

Apple Silicon M1 Max

$ uname -a
Darwin xxx.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64
./a.out
nan = nan
neg_nan = nan
small_value = 5.96046e-08

Amazon Linux 2023 AMI (64bit x86)

AMI = ami-0f403e3180720dd7e

[ec2-user@ip-172-30-0-119 ~]$ uname -a
Linux ip-172-30-0-119.ec2.internal 6.1.79-99.164.amzn2023.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Feb 27 18:02:23 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
[ec2-user@ip-172-30-0-119 ~]$ ./a.out 
nan = nan
neg_nan = -nan
small_value = 5.96046e-08

Amazon Linux 2023 AMI (64bit Arm)

AMI = ami-0237525b5672165b3

[ec2-user@ip-172-30-0-105 ~]$ uname -a
Linux ip-172-30-0-105.ec2.internal 6.1.79-99.164.amzn2023.aarch64 #1 SMP Tue Feb 27 18:02:03 UTC 2024 aarch64 aarch64 aarch64 GNU/Linux
[ec2-user@ip-172-30-0-105 ~]$ ./a.out 
nan = nan
neg_nan = -nan
small_value = 5.96046e-08

64bit Arm architecture does not always ignore the signature of NaN value. Apple Silicon is highly likely to ignore the NaN signature, so it cannot treat the negative NaN value. I'm still not sure exactly if it's a specific problem with Apple's Silicon architecture. But some Arm64 platform do not recognize the bit pattern of the negative NaN.

Reference: dart-lang/sdk#49423

[banach-space]

Thank you so much for following-up on this! 🙏🏻

I was able to reproduce your findings, which means that we know how to enable the test on AArch64. @Lewuathe, do you have the cycles to prepare a patch? Happy to do it myself, but I feel that you should be the one claiming the credit :)

Last, but not least - completely unexpected conclusion! I'm incredibly grateful for your analysis - otherwise this would've just stated unresolved for a very long time 😅

[Lewuathe]

@banach-space I feel like preparing the patch for this, but I think we may need to figure out the rationale behind this behavior. We have found that the issue occurs only on the Apple Silicon platform, but we are still determining if it's due to its specifications or other reasons. Therefore, we cannot guarantee that other AArch64 platforms will ensure the signs of NaN value. Some AArch64 platform may have the same special behavior on the signess NaN value.

If we can exclude the Apple Silicon platform from the supporting list of correctness.mlir, it would be enough for the time being. But we may want to clarify why we exclude Apple Silicon there. Just stating "Apple Silicon seems not to support the negative NaN" is enough?

[banach-space]

IMHO, there are two separate issues (admittedly, related):

1. correctness.mlir failing on Apple Silicon,
2. inconsistent behaviour with respect to -nan between Apple Silicon and AWS AArch64.

You have already determined the root cause for 1. and clearly that's something outside of MLIR (and LLVM) - hence my suggestion to:

• update correctness.mlir,
• close this ticket.

More specifically, 2. can only be addressed outside of LLVM/MLIR. That doesn't feel like something that should be tracked in LLVM. Unless I've missed something?

If we can exclude the Apple Silicon platform from the supporting list of correctness.mlir, it would be enough for the time being.

Note that we only really need to update a few CHECK lines, i.e. those testing for -nan, e.g.:

• https://github.com/llvm/llvm-project/blob/main/mlir/test/Integration/Dialect/Complex/CPU/correctness.mlir#L192

That would make perfect sense to me - based on the discussion here, those CHECK should depend on the effective triple used. In fact, correctness.mlir probably shouldn't assume identical output regardless of the triple.

There are two other options:

1. add target triple to mlir-cpu-runner invocation,
2. extract pow_test and move to e.g. test/Integration/Dialect/Complex/CPU/X86/correctness.mlir.

I don't really like 1. - that would require running mlir-cpu-runner through an emulator and hence a bit fiddly set-up. The other option would make a lot of sense - it sounds like pow_test is target specific anyway.

All in all, I see 3 options here. Each of these would be valid and would allow us to remove XFAIL and to close this ticket.

Do you have a preference? You're the expert on this, so I'm happy to follow your lead :) Also, please let me know if I'm missing some important context here. Thanks again for your great detective work!

[Lewuathe]

@banach-space Thank you for clearly illustrating the thought. That totally makes sense.

In fact, correctness.mlir probably shouldn't assume identical output regardless of the triple.

I agree. The correctness.mlir should be platform agnostic so it should not expect -nan here considering the output of signed NaN is not so essential here.

So it is pretty reasonable to take the option 1 that we are going to update the CHECK lines using -nan and remove XFAIL directive.

I'll prepare the patch to fix this and please take a look into that when you get a chance.