On Tue, 2024-07-09 at 20:21 -0600, Jeff Law wrote:
> 
> 
> On 7/9/24 8:14 PM, Xi Ruoyao wrote:
> > On Tue, 2024-07-09 at 16:10 -0700, Vineet Gupta wrote:
> > > On 7/3/24 21:35, Xi Ruoyao wrote:
> > > > On Sun, 2024-06-30 at 17:47 -0700, Vineet Gupta wrote:
> > > > >    - Don't hardcode SI in patterns, try to keep X to avoid potential
> > > > >      sign extension pitfalls. Implementation wise requires skipping
> > > > >      :MODE specifier in match_operand which is flagged as missing mode
> > > > >      warning.
> > > > I'm unsure about this.  GCC Internal says:
> > > > 
> > > > ‘isfinitem2’
> > > > Return 1 if operand 1 is a finite floating point number and 0 otherwise.
> > > > m is a scalar floating point mode. Operand 0 has mode SImode, and
> > > > operand 1 has mode m.
> > > > 
> > > > Likewise for isnormalm2.  BTW isinfm2 is missing in the page.
> > > > 
> > > > So per the doc SImode is required.  At least the doc should be updated
> > > > to say "operand 0 has an integer mode" or something if doing so is
> > > > intentionally allowed.
> > > 
> > > While the man page does say  these return int, for which SI mode whould
> > > suffice, I feel that keeping it X is more flexible.
> > > 
> > > So we should change the gcc documentation - so you want to send a patch ?
> > 
> > I do want to change it (it'll make implementing this for LoongArch
> > easier as well) but I don't know what we precisely allow here.  "Any
> > integer mode" or something?
> I'd lean towards any integer mode.  I would expect that most targets 
> would ultimately use their native word_mode.

Hmm it seems not a documentation-only change then.  For LoongArch if I
just use DImode for a 64-bit only version:

(define_int_iterator FCLASS_MASK [68 136 952])
(define_int_attr fclass_optab
  [(68  "isinf")
   (136 "isnormal")
   (952 "isfinite")])

(define_expand "<FCLASS_MASK:fclass_optab><ANYF:mode>2"
  [(match_operand:DI   0 "register_operand" "=r")
   (match_operand:ANYF 1 "register_operand" " f")
   (const_int FCLASS_MASK)]
  "TARGET_HARD_FLOAT && TARGET_64BIT"
  {
    rtx ft0 = gen_reg_rtx (SImode);
    rtx t0 = gen_reg_rtx (DImode);
    rtx mask = GEN_INT (<FCLASS_MASK>);

    emit_insn (gen_fclass_<ANYF:fmt> (ft0, operands[1]));
    emit_insn (gen_extend_insn (t0, ft0, DImode, SImode, 0)); 
    emit_move_insn (t0, gen_rtx_AND (DImode, t0, mask));
    emit_move_insn (operands[0], gen_rtx_NE (DImode, t0, const0_rtx));

    DONE;
  })

Then GCC refuses to expand the builtin:

$ cat t.c
int t(float x)
{
        return __builtin_isinf(x);
}
$ ./gcc/cc1 t.c -O2

results:

t:
        b       %plt(isinf)

This is absolutely not what we want...

-- 
Xi Ruoyao <xry...@xry111.site>
School of Aerospace Science and Technology, Xidian University

Reply via email to