This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PowerPC PR target/84154, fix floating point to small integer conversion regression
On Mon, Feb 05, 2018 at 07:54:58AM -0500, Michael Meissner wrote:
> On Mon, Feb 05, 2018 at 05:57:25AM -0600, Segher Boessenkool wrote:
> > On Thu, Feb 01, 2018 at 02:31:17PM -0500, Michael Meissner wrote:
> > > This patch fixes the optimization regression that occurred on GCC 7 where
> > > conversions from the various floating point types to small integers would at
> > > times generate a store and a load.
> >
> > [ snip big explanation; thanks for that! ]
> >
> > Could you merge the signed and unsigned patterns, using any_fix? Or is
> > there a reason that cannot work (other than that <su> unsigned_fix seems
> > buggy, it should say "u")?
>
> Well that's the way the instructions are. For traditional FPR instructions we
> have FCTIWZ vs. FCTIWUZ, while on the VSX side we have XSCVDPSXWS
> vs. XSCVDPUXWS. If you mean the name of the insn, I can change it if desired,
> but originally it was based on the FPR insn name.
We have
(define_code_attr su [(sign_extend "s")
(zero_extend "u")
(fix "s")
(unsigned_fix "s")
(float "s")
(unsigned_float "u")])
and "s" for unsigned_fix seems like it should be "u". Very surprising
otherwise (if this is needed in some case, it should just write "s" there
instead of "<su>").
> > Okay for trunk even without that (but please try). Also okay for 7 after
> > looking for fallout.
>
> In the past, I have found that combining code iterators with two mode iterators
> has a bug where it would use the wrong code iterator, so I just avoided doing
> that. I'll see if it is still a bug.
Hrm. If you have multiple iterators you often need to use ":" syntax,
and you might want that anyway because the precedence rules are non-obvious;
but you are hitting something else? Please open a PR if so :-)
Segher