Re: RFA: MIPS paired single support

[Richard Sandiford <rsandifo at redhat dot com>]

The whole tone of your message, and the use of phrases like "micro-manage",
suggests that you found my review unfair and overly anal.  Sorry if that's
the case.  I was just trying to be thorough.

James E Wilson <wilson@specifixinc.com> writes:
> On Tue, 2004-08-24 at 05:25, Richard Sandiford wrote:
>> Sorry, but I don't like this at all.  One of your main reasons for using
>> things like vec_merge rather than UNSPECs was that it was possible to
>> describe the operation accurately with standard rtl codes.  If you're
>> saying that it _isn't_ possible to describe some of the patterns accurately
>> with standard rtl codes, please use UNSPECs for those patterns until the
>> situation changes.
>
> I think you are headed in the wrong direction here.  There are a lot of
> problems in the middle end with the vector support.  If I have to find
> and fix them all before we can add proper vector patterns to the mips.md
> file, then this is a hopeless task.  I think the right thing to do with
> the mips.md file is be consistent with all of the other ports that
> define vector support, until such time as the middle end issues are
> worked out, and that is exactly what I have done.

Well, I'm still uneasy about this, since it seems to assume that the
endiannes problems will be sorted out before the middle-end makes more
use of vec_merge, but I'll it go.

> It is trivial to test the vec_extract pattern.  All you have to do is
> perform a V2SF divide for a non-SB-1 target.  No source code changes
> required.  I have done this, and it works.

In that case, OK.

> Also, you are assuming that if I can't find an obvious way to trigger,
> than there are none.  It may be that there are some I haven't found yet.

I was more thinking that the pattern was going in untested, but...

> It is possible to test this patch.  Disable the vec_init pattern, and
> then perform a divide for a non-SB-1 target.  I have done this, and the
> patterns works.

...in that case, OK.

> It is easy to add extra type attributes, but the required DFA scheduler
> changes makes the change tedious, and results in large patches.  My
> change to add the frdiv type attribute was almost 17K.  I have to add 4
> new type attributes to handle the recip/sqrt step instructions
> correctly.  The float vector patch is already too large (180K+).  I
> think it is a mistake to add another large patch into it.  

Fair enough.  Do it later.

>> You didn't seem to answer my main suggestion though, which is that the
>> MIPS_BUILTIN_s shouldn't be needed.  It's just an ordinary 0-based
>> enumeration.  For example, instead of:
>
> I didn't understand exactly what you were asking for, and there are so
> many other requests you are making.
>
> Note that the way this patch is written is similar to how other back
> ends work, i.e. it is following existing practice.  I am a little
> puzzled why you think the MIPS port should work differently than all
> other ports.  However, I think this is moot, as I probably won't have
> time to try changing this.

Because I look at this patch and see two big enumerations (CODE_FOR_*
and MIPS_BUILTIN_*) that seem to be enumerating essentially the same
thing.  I really would like to see the redundancy removed if at all
possible.

But if you feel that this requirement is unfair, go head and check in
the patch with the MIPS_BUILTIN_* enumeration still there.  I'll try to
find time to rework it later.

Richard