Skip to content
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

Header files mandate GNU extensions #71

Open
DavidC-75 opened this issue Jun 22, 2023 · 2 comments
Open

Header files mandate GNU extensions #71

DavidC-75 opened this issue Jun 22, 2023 · 2 comments

Comments

@DavidC-75
Copy link

Compiling with GNU extensions (-std=gnu++14) works fine.
However compiling without these extensions (-std=c++14) reveals a few issues in some header files.

mplapack/mplapack_utils__Float128.h: In function '_Float128 sign(_Float128, _Float128)':
551:22: error: call of overloaded 'abs(_Float128&)' is ambiguous
  551 |     mtmp = std::abs(a);
mplapack/mplapack_utils__Float128.h:580:47: error: unable to find numeric literal operator 'operator""Q'
  580 | inline _Float128 pi(_Float128 dummy) { return M_PIq; }
qd/dd_inline.h:288:22: error: exponent has no digits
  288 |   if (fabs(a.x[0]) > 0x1p+969) {

Many occurrences of the latter are reported in dd_inline.h. All occurrences seem to appear on lines that are not part of the original dd_inline.h file but on lines that have been introduced by the patching process.

All the above are errors and not just warnings. It would be good to fix the header files so GNU extensions are not mandated to be able to compile successfully.

@nakatamaho
Copy link
Owner

Thanks for your report. Currently, _Float128 is only supported via the GNU extension. The ISO/IEC TS 18661-3:2015 defines _Float128 for C, but not C++! There's no way to treat _Float128 officially for C++.

@DavidC-75
Copy link
Author

DavidC-75 commented Jun 22, 2023

Well, let's start with the easy one: qd/dd_inline.h
QD is an external library, that has nothing to do with MPLAPACK.
The dd_inline.h from the original QD library is perfectly compatible with plain C++14 without GNU extensions.
With the 'overflow patch' that's applied by MPLAPACK though, compatibility with plain C++14 is broken. Fixing the problem is probably trivial, but the GCC error message (exponent has no digits) is not really guiding me in the right direction and I could not figure it out myself. Any suggestions how to do this ?
By the way, could you elaborate a bit more on this patch, why is it necessary and and what does it do precisely ?

Now, regarding mplapack_utils__Float128.h
I'm not at all a skilled SW engineer, and I have honestly no idea what is specified and what is not. Even if C++ does not define _Float128, I do not really see why GNU extensions must be enabled.
Looking carefully at the error messages, it seems to me that something is suspicious indeed.

 error: call of overloaded 'abs(_Float128&)' is ambiguous
  551 |     mtmp = std::abs(a);

Shouldn't std::abs(a) simply be replaced by fabsq(a) for a cleaner implementation (in the case where ___MPLAPACK_WANT_LIBQUADMATH___ is defined at least, I'm not sure for the other cases) ? In fact, it's not clear to me why the same error is not generated when enabling GNU extensions, but that's a different discussion.

The last error message (unable to find numeric literal operator 'operator""Q') can be avoided by compiling with -fext-numeric-literals but it can also be avoided by a cosmetic change to the source code. Is it not more portable to replace M_PIq with (_Float128)M_PI ?

With the above two changes in mplapack_utils__Float128.h, I was able to compile with C++14 without GNU extensions.
Can you confirm it was the right thing to do ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants