Re: [PATCHv3] Optab: add isfinite_optab for __builtin_isfinite

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi,

on 2024/5/27 20:54, Richard Biener wrote:
> On Mon, May 27, 2024 at 11:37 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>>
>> Hi,
>>   This patch adds an optab for __builtin_isfinite. The finite check can be
>> implemented on rs6000 by a single instruction. It needs an optab to be
>> expanded to the certain sequence of instructions.
>>
>>   The subsequent patches will implement the expand on rs6000.
>>
>>   Compared to previous version, the main change is to specify acceptable
>> modes for the optab.
>> https://gcc.gnu.org/pipermail/gcc-patches/2024-May/652170.html
>>
>>   Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no
>> regressions. Is this OK for trunk?
>>
>> Thanks
>> Gui Haochen
>>
>> ChangeLog
>> optab: Add isfinite_optab for isfinite builtin
>>
>> gcc/
>>         * builtins.cc (interclass_mathfn_icode): Set optab to isfinite_optab
>>         for isfinite builtin.
>>         * optabs.def (isfinite_optab): New.
>>         * doc/md.texi (isfinite): Document.
>>
>>
>> patch.diff
>> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
>> index f8d94c4b435..b8432f84020 100644
>> --- a/gcc/builtins.cc
>> +++ b/gcc/builtins.cc
>> @@ -2459,8 +2459,9 @@ interclass_mathfn_icode (tree arg, tree fndecl)
>>        errno_set = true; builtin_optab = ilogb_optab; break;
>>      CASE_FLT_FN (BUILT_IN_ISINF):
>>        builtin_optab = isinf_optab; break;
>> -    case BUILT_IN_ISNORMAL:
>>      case BUILT_IN_ISFINITE:
>> +      builtin_optab = isfinite_optab; break;
>> +    case BUILT_IN_ISNORMAL:
>>      CASE_FLT_FN (BUILT_IN_FINITE):
>>      case BUILT_IN_FINITED32:
>>      case BUILT_IN_FINITED64:
>> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
>> index 5730bda80dc..67407fad37d 100644
>> --- a/gcc/doc/md.texi
>> +++ b/gcc/doc/md.texi
>> @@ -8557,6 +8557,15 @@ operand 2, greater than operand 2 or is unordered with operand 2.
>>
>>  This pattern is not allowed to @code{FAIL}.
>>
>> +@cindex @code{isfinite@var{m}2} instruction pattern
>> +@item @samp{isfinite@var{m}2}
>> +Set operand 0 to nonzero if operand 1 is a finite @code{SFmode},
>> +@code{DFmode}, or @code{TFmode} floating point number and to 0
> 
> It should probably say scalar floating-point mode?  But what about the result?
> Is any integer mode OK?  That's esp. important if this might be used on
> vector modes.
> 
>> +otherwise.
>> +
>> +If this pattern @code{FAIL}, a call to the library function
>> +@code{isfinite} is used.
> 
> Or it's otherwise inline expanded?  Or does this imply targets
> have to make sure to implement the pattern when isfinite is
> not available in libc/libm?  I suggest to leave this sentence out,
> we usually only say when a pattern may _not_ FAIL (and usually
> FAILing isn't different from not providing a pattern).

As Haochen's previous reply, I think there are three cases:
  1) no optab defined, fold in a generic way;
  2) optab defined, SUCC, expand as what it defines;
  3) optab defined, FAIL, generate a library call;

From above, I had the concern that ports may assume FAILing can
fall back with the generic folding, but it's not actually.
Does your comment imply ports usually don't make such assumption
(or they just check what happens for FAIL)?

BR,
Kewen

> 
>>  @end table
>>
>>  @end ifset
>> diff --git a/gcc/optabs.def b/gcc/optabs.def
>> index ad14f9328b9..dcd77315c2a 100644
>> --- a/gcc/optabs.def
>> +++ b/gcc/optabs.def
>> @@ -352,6 +352,7 @@ OPTAB_D (fmod_optab, "fmod$a3")
>>  OPTAB_D (hypot_optab, "hypot$a3")
>>  OPTAB_D (ilogb_optab, "ilogb$a2")
>>  OPTAB_D (isinf_optab, "isinf$a2")
>> +OPTAB_D (isfinite_optab, "isfinite$a2")
>>  OPTAB_D (issignaling_optab, "issignaling$a2")
>>  OPTAB_D (ldexp_optab, "ldexp$a3")
>>  OPTAB_D (log10_optab, "log10$a2")