Skip to content

[builtins] Avoid using long double in generic sources #69754

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.

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

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

arichardson
Copy link
Member

Use of long double can be error-prone since it could be one of 80-bit
extended precision float, IEEE 128-bit float, or IBM 128-bit float.
Instead use an explicit xf_float typedef for the remaining cases where
long double is being used in the implementation. This patch does not touch
the PPC specializations which still use long double.

Depends on #68132 (to be committed after that PR).

@github-actions
Copy link

github-actions bot commented Oct 20, 2023

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

@arichardson arichardson force-pushed the compiler-rt-no-long-double branch from 6298a09 to 679e906 Compare October 24, 2023 16:34
Use of long double can be error-prone since it could be one of 80-bit
extended precision float, IEEE 128-bit float, or IBM 128-bit float.
Instead use an explicit xf_float typedef for the remaining cases where
long double is being used in the implementation. This patch does not touch
the PPC specializations which still use long double.
@arichardson arichardson force-pushed the compiler-rt-no-long-double branch from 679e906 to 0af8ccd Compare October 24, 2023 16:57
@arichardson arichardson merged commit 05a4212 into llvm:main Oct 25, 2023
@arichardson arichardson deleted the compiler-rt-no-long-double branch October 25, 2023 00:15
@nico
Copy link
Contributor

nico commented Oct 30, 2023

This seems to not build on Windows, at least with clang-cl as host compiler: http://45.33.8.238/win/85105/step_4.txt

Could you take a look?

@arichardson
Copy link
Member Author

This seems to not build on Windows, at least with clang-cl as host compiler: http://45.33.8.238/win/85105/step_4.txt

Could you take a look?

My guess is that HAS_80_BIT_LONG_DOUBLE is not being defined for some reason?

#if (defined(__i386__) || defined(__x86_64__)) &&                              \
    !(defined(_MSC_VER) || defined(__ANDROID__))
#define HAS_80_BIT_LONG_DOUBLE 1
#elif defined(__m68k__) || defined(__ia64__)
#define HAS_80_BIT_LONG_DOUBLE 1
#else
#define HAS_80_BIT_LONG_DOUBLE 0
#endif

#if HAS_80_BIT_LONG_DOUBLE

@arichardson
Copy link
Member Author

So either clang-cl does not use 80 bit long double and we shouldn't be compiling those files or the guard needs to be adjusted. Not familiar with Windows ABIs so I'd have to defer to someone with experience there.

@pranavk
Copy link
Contributor

pranavk commented Oct 30, 2023

Yes, Windows long double is 64 bits, not 80 bits.

@arichardson
Copy link
Member Author

Yes, Windows long double is 64 bits, not 80 bits.

Does this mean the *xf files should not be compiled for Windows ABIs?

@pranavk
Copy link
Contributor

pranavk commented Oct 30, 2023

Yes, that would make sense to me. *xf pertains to 80 bit float as far as I know.

Microsoft's long double

@nico
Copy link
Contributor

nico commented Oct 31, 2023

Apologies, https://reviews.llvm.org/D82153 already took care of that a while ago. Config problem on that bot.

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.

5 participants