Re: [PATCH] Improve converting between 128-bit modes that use the same format

[Michael Meissner <meissner@linux.ibm.com>]

On Tue, Sep 06, 2022 at 05:22:11PM -0500, Segher Boessenkool wrote:
> Please do this.  It is the biggest problem I have with most of your
> patches: you seem to save up development of a week, and then send it out
> as big omnibus patch an hour or two before my weekend.  This is not
> ideal.

This is always going to be the case.  As I'm developing the larger patches,
there are usually at least 3 smaller problems wanting to get out.  I don't know
what these things are until I run into them.

With the long patch review cycle, I have to be working n patches ahead of what
is submitted just to keep busy.

I try my best to segreate the patches into smaller chunks, but at the end of
the day there are going to various changes to get to the larger goal.  I really
don't see any way around that.

Things don't always come in 3 line changes.  A lot of times I make the 3 line
change, and then I have to make several other changes to compensate for it
elsewhere.  If I submit smaller patches, things will break unless all of the
patches in the chain are committed.  And invariably you will then ask why this
particular change is needed, when it is needed by the next patch or the patch
after that.

> > > > +(define_expand "extendkfif2"
> > > > +  [(set (match_operand:IF 0 "gpc_reg_operand")
> > > > +	(float_extend:IF (match_operand:KF 1 "gpc_reg_operand")))]
> > > > +  "TARGET_FLOAT128_TYPE"
> > > > +{
> > > > +  rs6000_expand_float128_convert (operands[0], operands[1], false);
> > > > +  DONE;
> > > > +})
> > > 
> > > This does not belong here.
> > > 
> > > It really shouldn't *exist* at all: this is *not* a float_extend!  It is
> > > not converting to a wider mode (as required!), but not even to a mode of
> > > higher precision: both IFmode and KFmode can represent (finite, normal)
> > > numbers the other can not.
> > 
> > We know that TFmode (if -mabi=ieeelongdouble) and KFmode are the same, just
> > like TFmode (if -mabi=ibmlongdouble) and IFmode are the same.  But RTL does not
> > know that these modes use the same representation.  So to convert between them,
> > it needs to use either FLOAT_EXTEND or FLOAT_TRUNCATE, depending on which
> > precision each of the three modes have (i.e. rs6000-modes.h).  So you need
> > these conversions in RTL.
> 
> You have "extends" also when it is to a lower precision!
>
> Also, let me say this again: this needs to be solved.  We cannot make
> real progress as long as we keep pretending every pait of floating point
> modes can be ordered.

I have no plans to solve this.  I view it as an unsolvable problem that will
invariably lead to lots and lots of changes that will need buy in from the
other GCC developers.  If you can make the changes, fine.  But I view it as
work with what you have rather than trying to change the whole floating point
infrastructure within GCC.

> > Unfortunately, you can't just use SUBREG before register allocation is done.
> 
> Why not?  It isn't valid (in all cases) *after* RA, but before is fine.

I haven't tracked it down.  But I tend to think it is a whack-a-mole problem
where there will be 50+ small changes that are needed to get this to work.  And
then we get back to the issue that you need all of these changes.

> But you do not want a subreg, you need a conversion.
> 
> > > But it certainly does not belong here in the middle of no-op moves.
> 
> This is still very true.
> 
> > So for example, for IBM floating point my current patches are:
> 
> > But for the IEEE side, combining the two insns won't work, since going from
> > TFmode to KFmode will generate a FLOAT_TRUNCATE instead of a FLOAT_EXTEND.
> 
> Yes.  Please just fix the code depending on fundamentally wrong and
> unworkable assumptions, instead of adding perpetually worse workarounds
> that make things more rickety all the time.
> 
> When you added this IF/KF/TF ordering stuff first, I was amazed that it
> worked as well as it did.  And this was perhaps the best we could do for
> GCC 10, sure.  But the problem should have been fixed since, there is no
> sane way forward without doing that.

When I first looked at it, the number of hidden assumptions that I discovered
just in the changes I made was high.  I believe fundamentally that there will
be a lot more changes to do what you want.  And as I said, this will need a lot
of buy-in from other developers.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meissner@linux.ibm.com