On Thu, Apr 07, 2022 at 06:09:52AM -0500, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Mar 03, 2022 at 10:11:32AM +0800, Kewen.Lin wrote:
> > As PR103623 shows, it's a regression failure due to new built-in
> > function framework, previously we guard __builtin_{un,}pack_{longdouble,
> > ibm128} built-in functions under hard float, so they are unavailable
> > with the given configuration.  While with new bif infrastructure, it
> > becomes available and gets ICE due to incomplete supports.
> > 
> > Segher and Peter pointed out that we should make them available with
> > soft float, I agree we can extend them to cover both soft and hard
> > float.  But considering it's stage 4 now and this regression is
> > classified as P1, also the previous behavior requiring hard float
> > aligns with what document [1] says, I guess it may be a good idea to
> > fix it with the attached small patch to be consistent with the previous
> > behavior.  Then we can extend the functionality in upcoming stage 1.
> 
> Or you could just not take away the existing functionality.

Have those builtins ever worked with 64-bit soft-float?
When I try e.g. gcc 11 on the #c0 testcase from the PR, I get:
./cc1.r11-1 -quiet -nostdinc pr103623.c -m64 -mlong-double-128 -msoft-float
pr103623.c: In function ‘main’:
pr103623.c:2:16: error: ‘__builtin_unpack_longdouble’ requires the 
‘-mhard-float’ option
    2 | #define UNPACK __builtin_unpack_longdouble
      |                ^
pr103623.c:11:15: note: in expansion of macro ‘UNPACK’
   11 |   double x0 = UNPACK (a, 0);
      |               ^~~~~~

>From what I can see, those builtins were using:
/* 128-bit long double floating point builtins.  */
#define BU_LDBL128_2(ENUM, NAME, ATTR, ICODE)                           \
  RS6000_BUILTIN_2 (MISC_BUILTIN_ ## ENUM,              /* ENUM */      \
                    "__builtin_" NAME,                  /* NAME */      \
                    (RS6000_BTM_HARD_FLOAT              /* MASK */      \
                     | RS6000_BTM_LDBL128),                             \
                    (RS6000_BTC_ ## ATTR                /* ATTR */      \
                     | RS6000_BTC_BINARY),                              \
                    CODE_FOR_ ## ICODE)                 /* ICODE */
   
/* 128-bit __ibm128 floating point builtins (use -mfloat128 to indicate that
   __ibm128 is available).  */
#define BU_IBM128_2(ENUM, NAME, ATTR, ICODE)                            \
  RS6000_BUILTIN_2 (MISC_BUILTIN_ ## ENUM,              /* ENUM */      \
                    "__builtin_" NAME,                  /* NAME */      \
                    (RS6000_BTM_HARD_FLOAT              /* MASK */      \
                     | RS6000_BTM_FLOAT128),                            \
                    (RS6000_BTC_ ## ATTR                /* ATTR */      \
                     | RS6000_BTC_BINARY),                              \
                    CODE_FOR_ ## ICODE)                 /* ICODE */
macros and rs6000_builtin_is_supported_p was checking whether
all the bits in the mask are set:
  HOST_WIDE_INT fnmask = rs6000_builtin_info[fncode].mask;
  if ((fnmask & rs6000_builtin_mask) != fnmask)
    return false;
  else
    return true;
(so logical and of all of them).

        Jakub

Reply via email to