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)
