Re: Patch to change IA64 division code

[Jim Wilson <wilson at specifix dot com>]

On Tue, 2007-03-27 at 10:21 -0700, Steve Ellcey wrote:
> Here is a new version of my division change.  I got rid of the _a and _b
> variants and changed the division sequences to use gen_* calls.  This
> makes div.md smaller and easier to read.  The code sequence generated is
> unchanged.

Yes, this is looking much nicer.

I noticed that you are using 'U' to match FP constant 0 in the combined
_a/_b patterns.  However, 'U' is documented to be for a vector constant
0, and this isn't a vector constant.  So either we need to change the
comments for 'U', or we need to add a new letter.  There are two letters
reserved for FP constants, G and H, and we currently only use G.  So I
think it would be better to add a new constraint H that is for FP
constant 0.  (Or alternatively, we could modify G which is currently
both 0.0 and 1.0, and have G match 0.0 and H match 1.0, but that would
be more work.)

I should have been a bit more explicit when I asked about the
UNSPEC_NOP_CONVERT.  I don't understand why you aren't using the
standard float_extend and float_truncate operators.  You should only add
an unspec unless you have some actual need for an unspec, and I don't
see one here.  The comments you added say that you are just doing a FP
conversion, so just use the standard FP conversion RTL operators.  It is
OK to have an FP conversion that later splits into a simple move
instruction.  rs6000 for instance already has instances of that.  If use
of the standard conversion operators fails for some reason, then it
would be good to have that reason listed in the comments.  The use of an
UNSPEC unnecessarily here could result in nop move instructions that
won't get optimized away.

> I am still a bit concerned about HARD_REGNO_CALLER_SAVE_MODE.

FYI you still have the ChangeLog entry for this change, but it isn't in
this version of the patch.

You raise a good point here.  I had to look at the
HARD_REGNO_CALLER_SAVE_MODE code to remind myself how it works.  I think
it is OK to use RFmode here as you had it originally.  Since we already
have RFmode support via the __fpregs builtin type, this probably should
have been fixed earlier.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com