This is an archive of the discontinued LLVM Phabricator instance.

[Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and __builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode
ClosedPublic

Authored by xingxue on Nov 14 2022, 2:32 PM.

Details

Summary

AIX library functions frexpl(), ldexpl(), and modfl() are for 128-bit IBM long double, i.e. __ibm128. Other *l() functions, e.g., acosl(), are for 64-bit long double. The AIX Clang compiler currently maps builtin functions __builtin_frexpl(), __builtin_ldexpl(), and __builtin_modfl() to frexpl(), ldexpl(), and modfl() in 64-bit long double mode which results in seg-faults or incorrect return values. This patch changes to map __builtin_frexpl(), __builtin_ldexpl(), and __builtin_modfl() to double version lib functions frexp(), ldexp() and modf() in 64-bit long double mode. The following is from AIX <math.h>.

/*
 * frexpl(), ldexpl(), and modfl() have preexisting shared versions which are
 * 128-bit only.  64-bit versions must be made available for C99 for the
 * default 64-bit long double.  These cannot simply be macros because the
 * actual routines will be the incorrect form in 64-bit mode if the user
 * forces the actual routines to be used through undef or macro suppression
 * per the standard.
 */
#if defined(_ISOC99_SOURCE) && !defined(__LONGDOUBLE128)
static long double _NOTHROW(frexpl, (long double __x, int *__i))
{
     return (long double) frexp((double) __x, __i);
}

static long double _NOTHROW(ldexpl, (long double __x, int __i))
{
     return (long double) ldexp((double) __x, __i);
}

#ifndef __MODFL
static long double _NOTHROW(modfl, (long double __x, long double *__y))
{
     return (long double) modf((double) __x, (double *) __y);
}
#endif
#endif

Diff Detail

Event Timeline

xingxue created this revision.Nov 14 2022, 2:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 14 2022, 2:32 PM
xingxue requested review of this revision.Nov 14 2022, 2:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 14 2022, 2:32 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
clang/lib/CodeGen/CGBuiltin.cpp
112

Please rename to "AIXLongDouble64Builtins".

clang/test/CodeGen/aix-builtin-mapping.c
3

Fix typo.

5–6

This should be an IR test. Checking assembly means actually needing the PowerPC target enabled in the LLVM configuration.

daltenty added inline comments.Nov 16 2022, 9:06 AM
clang/lib/CodeGen/CGBuiltin.cpp
110

I feel like we should be clear about which 128-bit double format we are talking about, since it may not be immediately clear for folks who aren't intimately familiar with AIX. Maybe we can reference __ibm128 or ibm 128-bit, so it's clear what we mean.

clang/test/CodeGen/aix-builtin-mapping.c
2
xingxue updated this revision to Diff 475853.Nov 16 2022, 9:52 AM
xingxue marked 5 inline comments as done.
xingxue edited the summary of this revision. (Show Details)

Addressed comments:

  • rename AIXLongDoubleBuiltins to AIXLongDouble64Builtins
  • test IR output instead of assembly output
  • fix a typo in a comment
  • mention frexpl(), ldexpl(), and modfl() are for 128-bit IBM long double, i.e. __ibm128
xingxue added inline comments.Nov 16 2022, 10:46 AM
clang/lib/CodeGen/CGBuiltin.cpp
110

Modified as suggested, thanks!

112

Renamed as suggested, thanks!

clang/test/CodeGen/aix-builtin-mapping.c
2

Modified as suggested, thanks!

3

Fixed, thanks for catching it!

5–6

Changed to use IR, thanks!

This revision is now accepted and ready to land.Nov 16 2022, 3:57 PM
daltenty accepted this revision.Nov 18 2022, 7:55 AM

LGTM, thanks!