Re: [MIPS] MADD issue

[Nigel Stephens <nigel at mips dot com>]

Richard Sandiford wrote:
> Nigel Stephens <nigel@mips.com> writes:
>   
> > I notice that at least the 32-bit rs6000, i386, sparc, frv, sh, cris, 
> > mcore, score, arm & pa backends still implement adddi3 as either a 
> > define_insn which outputs two instructions or an explicit define_expand 
> > followed define_split and associated sub patterns.  Are we setting the 
> > bar too high for MIPS? :)
> >     
>
> I don't think that follows.  The main reason that ports like rs6000,
> i386, arm, sparc and pa define adddi3 is because those architectures
> provide special add-with-carry type instructions, or similar
> architecture-specific optimisations.  

Right, good point.

> MIPS has nothing like that.
>   

Actually the MIPS DSP ASE does have addsc and addwc, which could be used 
for this purpose. Sadly not subsc and subwc, though.

> The old MIPS patterns just re-implemented the standard optabs version
> (and often did so less efficiently, as I said before).
>
>   
> > Whilst I'm sure that your proposal is the right one going forward, it 
> > still feels like it could be significant amount of work to implement. 
> > And the simplified optab/expand support would only work for multiply-add 
> > sequences expressed within a single expression, and wouldn't be able to 
> > optimise disjoint multiply, then add expressions. Or have I missed 
> > something.
> >     
>
> I don't think that follows either.  Out-of-ssa should coalesce them
> if they are in the same basic block.  And if they aren't in the
> same basic block, that's presumably because the tree optimisers
> think they shouldn't be.  Combine wouldn't look across basic
> block boundaries either AFAIK.
>
> E.g. compiling:
>
> --------------------------------------------------------------------
> typedef unsigned long long ull;
> typedef unsigned int ui;
>
> ull foo (ui x, ui y, ull z)
> {
>   ull tmp = (ull) x * y;
>   return tmp + z;
> }
> --------------------------------------------------------------------
>
> with a recent snapshot and "-O2 -fdump-tree-all" shows that the
> final_cleanup dump does indeed have the combined form:
>
> --------------------------------------------------------------------
> ;; Function foo (foo)
>
> foo (x, y, z)
> {
> <bb 2>:
>   return (long long unsigned int) y * (long long unsigned int) x + z;
>
> }
> --------------------------------------------------------------------
>
>   

Ah I see. Fair enough.

> I realise no-one else has spoken out in support of me, so perhaps
> I'm in a minority of one here.  But it does seem to me that in the
> Tree-SSA world, it makes less sense to duplicate standard optabs
> in the backend purely for the reason of keeping DImode arithmetic
> around as DImode arithmetic for longer.
>
>   
> > in the short term we really do need to reenable madd/msub for MIPS32 
> > targets in GCC 4. We could do that with a local patch to put back 
> > adddi3, but it would be better if we could coordinate this with you.
> >     
>
> If removing the patterns had been purely a clean-up, I would be more
> open to the idea of putting the patterns back.  But given that removing
> the patterns had an optimisation benefit of its own, I'm less open to
> the idea of adding them back, especially when (as far as I'm concerned)
> there's a clean way of getting the best of both worlds.
>
>   

OK, so maybe as the person who removed adddi3 from the MIPS backend, and 
the main  proponent of the new fused opcodes, you get to volunteer to 
implement this? :)

In the meantime the performance gain from being able to use a widening 
madd is more important to us than the benefit of improved optimisation 
of 64-bit addition, so we'll probably have to put adddi3 back in as a 
local patch.

Nigel