RE: PING**4 - [PATCH] middle-end: Support ABIs that pass FP values as wider integers.

["Roger Sayle" <roger@nextmovesoftware.com>]

Hi Richard,

> The above seems backwards - it should be (subreg:HF (reg:SI 0)), no?

Line 949 of emit-rtl.cc in the definition of "validate_subreg" contains:

  /* Subregs involving floating point modes are not allowed to
     change size.  Therefore (subreg:DI (reg:DF) 0) is fine, but
     (subreg:SI (reg:DF) 0) isn't.  */
  else if (FLOAT_MODE_P (imode) || FLOAT_MODE_P (omode))
    {
      if (! (known_eq (isize, osize)

Hence, we ICE on line 1022 of emit-rtl.cc in gen_rtx_SUBREG, that asserts
validate_subreg, when called with omode=HFmode and imode=SImode.

Cheers,
Roger
--

> -----Original Message-----
> From: Richard Biener <rguenther@suse.de>
> Sent: 14 March 2022 10:15
> To: Roger Sayle <roger@nextmovesoftware.com>
> Cc: 'Tom de Vries' <tdevries@suse.de>; 'Jeff Law' <jeffreyalaw@gmail.com>;
> 'Tobias Burnus' <tobias@codesourcery.com>; richard.sandiford@arm.com;
'Jeff
> Law' <law@redhat.com>; gcc-patches@gcc.gnu.org
> Subject: RE: PING**4 - [PATCH] middle-end: Support ABIs that pass FP
values as
> wider integers.
> 
> On Mon, 14 Mar 2022, Roger Sayle wrote:
> 
> >
> > Hi Richard,
> > Thanks for asking.
> > The issue is with 16-bit floating point HFmode, where clang on nvptx
> > passes HFmode values in SImode registers, and for binary compatibility
> > GCC needs to do the same.
> > Their motivation is that for compatibility with older GPUs and the
> > x86_64 host, HFmode parameters are treated like "unsigned int".
> > Hence, libgcc functions like __sqrthf behave as though declared (are
> > binary compatible with) "unsigned short __sqrthf(unsigned short)".
> > As you point out, this also greatly simplifies soft-float, as both
> > ABIs remain the same.
> >
> > Alas, the subreg approach doesn't work as we'd end up with (subreg:SI
> > (subreg:HI (reg:HF)), though technically that is what this patch does,
> > inserting a pair of conversion instructions.
> 
> So what does FUNCTION_ARG return for the HFmode parameters?  The above
> seems backwards - it should be (subreg:HF (reg:SI 0)), no?
> 
> Or are we talking about the caller side?  Or the return value case (in
which case I
> ask the same question wrt FUNCTION_VALUE)?
> 
> Does nvptx use the promotion hooks in those cases?
> 
> > Until recently (subreg:SI (reg:HF)) did work, but that support was
> > removed/cleaned-up,
> > as we need sensible subreg semantics in the RTL passes.   The proposed
patch
> > simply
> > adds support back in the minimal places where changing FP/non-FP and
> > precision at the same time is required: argument passing and return
> > values.
> >
> > Hopefully this answers your question.  An alternate viewpoint might be
> > "is there a reason for GCC not to be able to support targets with
> > slightly wacky parameter passing conventions".
> >
> > Thanks for thinking about this.
> > Roger
> > --
> >
> > > -----Original Message-----
> > > From: Richard Biener <rguenther@suse.de>
> > > Sent: 14 March 2022 09:09
> > > To: Roger Sayle <roger@nextmovesoftware.com>
> > > Cc: 'Tom de Vries' <tdevries@suse.de>; 'Jeff Law'
> > > <jeffreyalaw@gmail.com>; 'Tobias Burnus' <tobias@codesourcery.com>;
> > > richard.sandiford@arm.com;
> > 'Jeff
> > > Law' <law@redhat.com>; gcc-patches@gcc.gnu.org
> > > Subject: RE: PING**4 - [PATCH] middle-end: Support ABIs that pass FP
> > values as
> > > wider integers.
> > >
> > > On Mon, 14 Mar 2022, Roger Sayle wrote:
> > >
> > > >
> > > > I thought I'd add a few comments that might help reviewers of this
> > patch.
> > > > Most importantly, this patch should be completely safe, as both
> > > > changes only trigger with FLOAT vs INT size mismatches which
> > > > currently both ICE in the compiler a few lines later.  Admittedly,
> > > > this indicates something odd about a target's choice of ABI, but
> > > > one alternative might be to issue a "sorry, unimplemented" error
> > > > message rather than ICE, perhaps with a TODO or FIXME comment that
> > > > support for mixed size FP/integer ABIs be added in future.
> > > >
> > > > The only (other?) possible point of contention is the (arbitrary)
> > > > decision that when passing floating point values in a larger
> > > > integer register, the code is hardwired to use zero-extension.
> > > > This in theory could be turned into a target hook to support
> > > > sign-extension, but given these are padding bits, zero seems
> > > > appropriate. [On x86_64, if passing DFmode argument in a V2DFmode
> > > > vector, say, it seems reasonable
> > to
> > > use movq and zero the high bits].
> > > >
> > > > The final point is that at the moment, the only affected target is
> > > > nvptx-none, as I don't believe any other backend specifies an ABI
> > > > that requires passing floating point values in wider integer
registers.
> > > > Having said that, most targets don't yet support HFmode, and their
> > > > ABI specifications haven't yet been updated as what to do in that
> > > > eventuality.  If they choose to treat these like HImode, they'll
> > > > run into the same issues, as most ABIs pass HImode in wider
> > > > word_mode
> > registers.
> > > >
> > > > I hope this helps.  If folks could air their concerns out loud, I
> > > > can try my best to address those worries.
> > >
> > > First of all I'm not familiar with the ABI related code as all, so I
> > refrained from
> > > commenting.  But now I've looked closer (still unfamiliar code).
> > >
> > > I suppose there's targets passing SFmode in a SImode GPR and DFmode
> > > in a DImode GPR (all soft-float targets?), so that already works
somehow.
> > > Why does nvptx choose DImode for SFmode passing then?  Can't it
> > > choose (subreg:SI di:..) or so?  Does it require zero-extending to
> > > DImode on the
> > caller
> > > side?  It seems your expand_expr_real_1 code does not rely on that?
> > > So,
> > why
> > > does nvptx function_arg hook (?) insist on returning a DImode reg
> > > for an
> > SFmode
> > > argument rather than an SImode subreg of that?
> > >
> > > Richard.
> > >
> > > >
> > > > Many thanks in advance (and thanks to Tobias and Tom for pushing
> > > > for
> > this).
> > > > Roger
> > > > --
> > > >
> > > > > -----Original Message-----
> > > > > From: Tom de Vries <tdevries@suse.de>
> > > > > Sent: 14 March 2022 08:06
> > > > > To: Jeff Law <jeffreyalaw@gmail.com>; Richard Biener
> > > > > <rguenther@suse.de>; Tobias Burnus <tobias@codesourcery.com>
> > > > > Cc: richard.sandiford@arm.com; Jeff Law <law@redhat.com>; gcc-
> > > > > patches@gcc.gnu.org; Roger Sayle <roger@nextmovesoftware.com>
> > > > > Subject: PING**4 - [PATCH] middle-end: Support ABIs that pass FP
> > > > > values as wider integers.
> > > > >
> > > > > On 3/2/22 20:18, Jeff Law via Gcc-patches wrote:
> > > > > >
> > > > > >
> > > > > > On 2/28/2022 5:54 AM, Richard Biener via Gcc-patches wrote:
> > > > > >> On Mon, 28 Feb 2022, Tobias Burnus wrote:
> > > > > >>
> > > > > >>> Ping**3
> > > > > >>>
> > > > > >>> On 23.02.22 09:42, Tobias Burnus wrote:
> > > > > >>>> PING**2 for the ME review or at least comments to that
> > > > > >>>> patch, which fixes a build issue/ICE with nvptx
> > > > > >>>>
> > > > > >>>> Patch:
> > > > > >>>>
https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590139.
> > > > > >>>> html (for gcc/cfgexpand.cc + gcc/expr.cc)
> > > > > >>>>
> > > > > >>>> (There is some discussion by Tom and Roger about the BE in
> > > > > >>>> the patch thread, which only not relate to the ME patch.
> > > > > >>>> But there is no ME-patch comment so far.)
> > > > > >>> The related BE patch has been already committed, but to be
> > > > > >>> effective, it needs the ME patch.
> > > > > >> I'm not sure I'm qualified to review this - maybe Richard is.
> > > > > > I'd initially ignored the patch as it didn't seem a good fit
> > > > > > for stage4, subsequent messages changed my mind about it, but
> > > > > > I never went back to take a deeper look at Roger's patch.
> > > > >
> > > > > Ping.
> > > > >
> > > > > [ FWIW, I'd appreciate it if a response came before the end of
> > > > > stage 4, such that I have some time left to deal with fallout in
> > > > > case the patch is not approved. ]
> > > > >
> > > > > Thanks,
> > > > > - Tom
> > > >
> > >
> > > --
> > > Richard Biener <rguenther@suse.de>
> > > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > > Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
> >
> >
> 
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)