Skip to content

Conversation

mstorsjo
Copy link
Member

This fixes the ifdefs added in
e9e166e; we need to include int_lib.h first before we can expect these defines to be set.

Also remove the XFAILs for aarch64 windows. As this test now became a no-op on platforms that lack CRT_HAS_128BIT or CRT_HAS_F128 (aarch64 windows lacks the latter), it no longer fails.

@mstorsjo
Copy link
Member Author

This should fix failing tests that I've run into in my nightly build at https://github.com/mstorsjo/llvm-mingw/actions/runs/17993746384/job/51211649698.

Copy link

github-actions bot commented Sep 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

This fixes the ifdefs added in
e9e166e; we need to include
int_lib.h first before we can expect these defines to be set.

Also remove the XFAILs for aarch64 windows. As this test now
became a no-op on platforms that lack CRT_HAS_128BIT or
CRT_HAS_F128 (aarch64 windows lacks the latter), it no longer
fails.
@mstorsjo mstorsjo force-pushed the compiler-rt-fix-aarch64-builtins-test branch from c7c4d11 to d851e31 Compare September 25, 2025 11:31
@DavidSpickett
Copy link
Collaborator

The code is new to me but after inspecting the headers, the change makes sense.

I'll let @ahatanak take a look too.

Copy link
Collaborator

@ahatanak ahatanak left a comment

Choose a reason for hiding this comment

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

Ah, I didn't realize I'd unconditionally disabled the tests. Thanks for catching the error.

@mstorsjo mstorsjo merged commit 6567070 into llvm:main Sep 26, 2025
9 checks passed
@mstorsjo mstorsjo deleted the compiler-rt-fix-aarch64-builtins-test branch September 26, 2025 08:49
@alazarev
Copy link
Contributor

@mstorsjo
Copy link
Member Author

It breaks https://lab.llvm.org/buildbot/#/builders/66/builds/19787

Hmm, right. So this is essentially e9e166e which enabled these tests to run in these configurations (where they didn't use to run before), where they now show up as broken.

I guess the best first step is to revert both this and e9e166e, and then @ahatanak can look into what's needed to make this configuration not break, if they would be relanded.

@mstorsjo
Copy link
Member Author

I pushed a revert in f7e8350.

@rorth
Copy link
Collaborator

rorth commented Sep 26, 2025

The test also breaks the Solaris/amd64 bot:

  • FAIL: Builtins-x86_64-sunos :: fixunstfdi_test.c:
    Exit Code: 1
    
    Command Output (stdout):
    --
    error in __fixunstfdi(0X1.0000000000000000P+0) = 0, expected 1
    
  • FAIL: Builtins-x86_64-sunos :: multc3_test.c:
    /var/llvm/tools/clang/stage2-bins/projects/compiler-rt/test/builtins/Unit/X86_64SunOSConfig/Output/multc3_test.c.script: line 1: 18353 Segmentation Fault
    

@ahatanak
Copy link
Collaborator

The signature of the function declared in the test file is different from the builtin's signature.

declaration in the test: long double _Complex __multc3(long double __a, long double __b, long double __c, long double __d);
builtin: Qcomplex __multc3(fp_t a, fp_t b, fp_t c, fp_t d).

I think we should skip the test if the return type or the argument types don't match.

ahatanak pushed a commit to ahatanak/llvm-project that referenced this pull request Sep 29, 2025
This fixes the ifdefs added in
e9e166e; we need to include int_lib.h
first before we can expect these defines to be set.

Also remove the XFAILs for aarch64 windows. As this test now became a
no-op on platforms that lack CRT_HAS_128BIT or CRT_HAS_F128 (aarch64
windows lacks the latter), it no longer fails.
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
This fixes the ifdefs added in
e9e166e; we need to include int_lib.h
first before we can expect these defines to be set.

Also remove the XFAILs for aarch64 windows. As this test now became a
no-op on platforms that lack CRT_HAS_128BIT or CRT_HAS_F128 (aarch64
windows lacks the latter), it no longer fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants