Skip to content

Conversation

@erichkeane
Copy link
Contributor

The previous check works for the EDG-based Intel compiler ONLY, but
Clang and GCC both have the -mlong-double-80 options (see
https://godbolt.org/z/h7s4Te). This patch generalizes the 80 bit check
to work with any compiler that defines the LDBL_DIG macro.

There are a couple of other macro options that are equally as opaque
(such as LDBL_MANT_DIG and LDBL_DECIMAL_DIG), but this seemed
the most general. Note that it ALSO works in ICC, as well as all Clang
derivatives (such as Intel's DPCPP).

The previous check works for the EDG-based Intel compiler ONLY, but
Clang and GCC both have the -mlong-double-80 options (see
https://godbolt.org/z/h7s4Te). This patch generalizes the 80 bit check
to work with any compiler that defines the __LDBL_DIG__ macro.

There are a couple of other macro options that are equally as opaque
(such as __LDBL_MANT_DIG__ and __LDBL_DECIMAL_DIG__), but this seemed
the most general. Note that it ALSO works in ICC, as well as all Clang
derivatives (such as Intel's DPCPP).
@erichkeane erichkeane requested a review from a team as a code owner March 10, 2021 15:16
@erichkeane
Copy link
Contributor Author

Note, this might be of interest to @simmse as this is a follow up to #1316.

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Mar 10, 2021
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

We don't want to invest much time into evaluating changes to semi-supported machinery like this, but since this doesn't increase complexity nor break the intended use case I suppose the change is fine.

@StephanTLavavej StephanTLavavej self-assigned this Mar 22, 2021
@StephanTLavavej StephanTLavavej merged commit 87fa813 into microsoft:main Mar 23, 2021
@StephanTLavavej
Copy link
Member

Congratulations on your first microsoft/STL commit! 🎉 😸 🚀

@erichkeane
Copy link
Contributor Author

@StephanTLavavej : Do you have any idea which release this fix should be included in?

Thanks!
-Erich

@CaseyCarter
Copy link
Contributor

@StephanTLavavej : Do you have any idea which release this fix should be included in?

If you search the https://github.com/microsoft/STL/wiki/Changelog for "1728" (the PR number) you'll see the change listed in the "Expected in VS 2019 16.10 Preview 3" section.

@erichkeane
Copy link
Contributor Author

@StephanTLavavej : Do you have any idea which release this fix should be included in?

If you search the https://github.com/microsoft/STL/wiki/Changelog for "1728" (the PR number) you'll see the change listed in the "Expected in VS 2019 16.10 Preview 3" section.

Perfect, thank you so much for the prompt answer!

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

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants