Hi Richard,
> Yes, which is why I think the target should claim argument passing happens
in reg:HI.
Unfortunately, this hits another "feature" of the nvptx backend; it's a
/* Implement TARGET_MODES_TIEABLE_P. */
bool nvptx_modes_tieable_p (machine_mode, machine_mode) { return false; }
/* Implement TARGET_CAN_CHANGE_MODE_CLASS. */
bool nvptx_can_change_mode_class (machine_mode, machine_mode, reg_class_t) {
return false; }
/* Implement TARGET_TRULY_NOOP_TRUNCATION. */
bool nvptx_truly_noop_truncation (poly_uint64, poly_uint64) { return false;
}
Basically, HImode is considered as a different register (bank) to SImode,
and requires
explicit move instructions to move data from HImode to SImode, and back,
unlike
most targets that can simply re-interpret the contents of GPRs.
Passing an argument as HImode would be incompatible with the requirement to
pass as SImode.
Cheers,
Roger
--
> -----Original Message-----
> From: Richard Biener <rguent...@suse.de>
> Sent: 14 March 2022 13:28
> To: Roger Sayle <ro...@nextmovesoftware.com>
> Cc: 'Tom de Vries' <tdevr...@suse.de>; 'Jeff Law' <jeffreya...@gmail.com>;
> 'Tobias Burnus' <tob...@codesourcery.com>; richard.sandif...@arm.com;
'Jeff
> Law' <l...@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,
> >
> > > 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.
>
> Yes, which is why I think the target should claim argument passing happens
in
> reg:HI. But as said, I'm not very familiar with argument passing
internals. But
> still the patch looks quite bolted on.
>
> For the expand_value_return I have to guess that 'val' has HFmode, the
> DECL_RESULT also has HFmode(?), promote_function_mode then returns
> SImode which isn't supported. So make it return HImode?
>
> > Cheers,
> > Roger
> > --
> >
> > > -----Original Message-----
> > > From: Richard Biener <rguent...@suse.de>
> > > Sent: 14 March 2022 10:15
> > > To: Roger Sayle <ro...@nextmovesoftware.com>
> > > Cc: 'Tom de Vries' <tdevr...@suse.de>; 'Jeff Law'
> > > <jeffreya...@gmail.com>; 'Tobias Burnus' <tob...@codesourcery.com>;
> > > richard.sandif...@arm.com;
> > 'Jeff
> > > Law' <l...@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 <rguent...@suse.de>
> > > > > Sent: 14 March 2022 09:09
> > > > > To: Roger Sayle <ro...@nextmovesoftware.com>
> > > > > Cc: 'Tom de Vries' <tdevr...@suse.de>; 'Jeff Law'
> > > > > <jeffreya...@gmail.com>; 'Tobias Burnus'
> > > > > <tob...@codesourcery.com>; richard.sandif...@arm.com;
> > > > 'Jeff
> > > > > Law' <l...@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 <tdevr...@suse.de>
> > > > > > > Sent: 14 March 2022 08:06
> > > > > > > To: Jeff Law <jeffreya...@gmail.com>; Richard Biener
> > > > > > > <rguent...@suse.de>; Tobias Burnus <tob...@codesourcery.com>
> > > > > > > Cc: richard.sandif...@arm.com; Jeff Law <l...@redhat.com>;
> > > > > > > gcc- patc...@gcc.gnu.org; Roger Sayle
> > > > > > > <ro...@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 <rguent...@suse.de> SUSE Software Solutions
> > > > > Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF:
> > > > > Ivo Totev; HRB 36809 (AG Nuernberg)
> > > >
> > > >
> > >
> > > --
> > > Richard Biener <rguent...@suse.de>
> > > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409
> > > Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
> >
> >
>
> --
> Richard Biener <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
