Rename conflicting macros in fenv.h and ieeefp.h

[dim]

Summary

During compilation tests with gcc, I noticed that several macros from
fenv.h and ieeefp.h are conflicting, e.g.:

In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13,
                 from /usr/src/lib/msun/tests/exponential_test.c:41:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:111:1: error: "__fldcw" redefined
In file included from /usr/src/lib/msun/tests/exponential_test.c:35:
/usr/obj/usr/src/tmp/usr/include/fenv.h:98:1: error: this is the location of the previous definition
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13,
                 from /usr/src/lib/msun/tests/exponential_test.c:41:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:112:1: error: "__fldenv" redefined
In file included from /usr/src/lib/msun/tests/exponential_test.c:35:
/usr/obj/usr/src/tmp/usr/include/fenv.h:99:1: error: this is the location of the previous definition
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13,
                 from /usr/src/lib/msun/tests/exponential_test.c:41:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:114:1: error: "__fnstcw" redefined
In file included from /usr/src/lib/msun/tests/exponential_test.c:35:
/usr/obj/usr/src/tmp/usr/include/fenv.h:105:1: error: this is the location of the previous definition
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13,
                 from /usr/src/lib/msun/tests/exponential_test.c:41:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:115:1: error: "__fnstenv" redefined
In file included from /usr/src/lib/msun/tests/exponential_test.c:35:
/usr/obj/usr/src/tmp/usr/include/fenv.h:104:1: error: this is the location of the previous definition
In file included from /usr/obj/usr/src/tmp/usr/include/ieeefp.h:13,
                 from /usr/src/lib/msun/tests/exponential_test.c:41:
/usr/obj/usr/src/tmp/usr/include/machine/ieeefp.h:116:1: error: "__fnstsw" redefined
In file included from /usr/src/lib/msun/tests/exponential_test.c:35:
/usr/obj/usr/src/tmp/usr/include/fenv.h:106:1: error: this is the location of the previous definition
*** [exponential_test.o] Error code 1

make[6]: stopped in /usr/src/lib/msun/tests

Unfortunately the macros in fenv.h and ieeefp.h are *not* compatible, as
the former usually take direct variables, while the latter take pointers.

In rS321483, @ngie worked around this by disabling warnings on macro
redefinitions, stating "this is a bandaid until the code is fixed and
will be reverted before MFC", but obviously that never happened.

To properly fix this, I propose to prefix the clashing macros from
fenv.h with __fenv, and those from ieeefp.h with __ieeefp, so the
headers can both be included without conflicts.

Test Plan

build universe

[kib]

Do arg-less macro conflict ? I think that no, and then there is no need to rename, use the __HAVE_MACRO_NAME trick to detect the presence of the macro in the namespace.

For the macros which conflict, IMO it is not too hard to convert other half of the users to the common interface. Since __ieeefp_XXX is used directly for the definition of the user-visible symbols, perhaps this interface should stay. You are changing every line which use the macros anyway.

If you need the help with assembly, ask.

[dim]

In D15633#330225, @kib wrote:

Do arg-less macro conflict ? I think that no, and then there is no need to rename, use the __HAVE_MACRO_NAME trick to detect the presence of the macro in the namespace.

I don't think they do, but it's also more complicated to do it this way.

For the macros which conflict, IMO it is not too hard to convert other half of the users to the common interface. Since __ieeefp_XXX is used directly for the definition of the user-visible symbols, perhaps this interface should stay. You are changing every line which use the macros anyway.

Maybe, but that will create a dependency of lib/msun on ieeefp.h, I'm not sure if that is OK with the msun maintainer(s), if there are still any.

[kib]

In D15633#330243, @dim wrote:

In D15633#330225, @kib wrote:

Do arg-less macro conflict ? I think that no, and then there is no need to rename, use the __HAVE_MACRO_NAME trick to detect the presence of the macro in the namespace.

I don't think they do, but it's also more complicated to do it this way.

Why do you consider it more complicated ?

For the macros which conflict, IMO it is not too hard to convert other half of the users to the common interface. Since __ieeefp_XXX is used directly for the definition of the user-visible symbols, perhaps this interface should stay. You are changing every line which use the macros anyway.

Maybe, but that will create a dependency of lib/msun on ieeefp.h, I'm not sure if that is OK with the msun maintainer(s), if there are still any.

Macros definitions could go into the separate header, included by both ieeefp.h and fenv.h.

[dim]

In D15633#330244, @kib wrote:

In D15633#330243, @dim wrote:

In D15633#330225, @kib wrote:

Do arg-less macro conflict ? I think that no, and then there is no need to rename, use the __HAVE_MACRO_NAME trick to detect the presence of the macro in the namespace.

I don't think they do, but it's also more complicated to do it this way.

Why do you consider it more complicated ?

Renaming macros is simple, and can be done mechanically, while adding conditionals is more error prone, and must be done much more carefully.

For the macros which conflict, IMO it is not too hard to convert other half of the users to the common interface. Since __ieeefp_XXX is used directly for the definition of the user-visible symbols, perhaps this interface should stay. You are changing every line which use the macros anyway.

Maybe, but that will create a dependency of lib/msun on ieeefp.h, I'm not sure if that is OK with the msun maintainer(s), if there are still any.

Macros definitions could go into the separate header, included by both ieeefp.h and fenv.h.

It would still make msun depend on something outside its lib directory, so I don't think that would gain much. I guess ieeefp.h is already our "internal" floating point handling header, so maybe it's best to use that.

[kib]

In D15633#330245, @dim wrote:

In D15633#330244, @kib wrote:

In D15633#330243, @dim wrote:

In D15633#330225, @kib wrote:

Do arg-less macro conflict ? I think that no, and then there is no need to rename, use the __HAVE_MACRO_NAME trick to detect the presence of the macro in the namespace.

I don't think they do, but it's also more complicated to do it this way.

Why do you consider it more complicated ?

Renaming macros is simple, and can be done mechanically, while adding conditionals is more error prone, and must be done much more carefully.

I do not follow, which conditions do you mean ? They are simply plug into the place.

For the macros which conflict, IMO it is not too hard to convert other half of the users to the common interface. Since __ieeefp_XXX is used directly for the definition of the user-visible symbols, perhaps this interface should stay. You are changing every line which use the macros anyway.

Maybe, but that will create a dependency of lib/msun on ieeefp.h, I'm not sure if that is OK with the msun maintainer(s), if there are still any.

Macros definitions could go into the separate header, included by both ieeefp.h and fenv.h.

It would still make msun depend on something outside its lib directory, so I don't think that would gain much. I guess ieeefp.h is already our "internal" floating point handling header, so maybe it's best to use that.

msun already depends on sys/ and machine/ headers. I am fine with locating the common header e.g. in sys/x86/include (and may even reuse it for sys/amd64/amd64/fpu.c and sys/i386/i386/npx.c after all.

[dim]

Completely revise approach, by letting lib/msun include <ieeefp.h>,
and defining all the common macros in the amd64 and i386 versions of
ieeefp.h instead. Those macros now use pointers as arguments, and
call sites have been updated.

After this, the only macros that are still "local" to fenv.h are now
__fldenvx and __fwait. Maybe these should also be moved to
ieeefp.h, but it is not strictly necessary; let me know if there is
any preference.

Last but not least this brings the amd64 and i386 versions of ieeefp.h
a little closer together, maybe as a follow-up these can be merged into
one x86 version?

[kib]

Thank you for doing that.

If the header becomes similar, of course it is useful to merge it.