[08/11] [rs6000] Fix ambiguous .md attribute uses

[Segher Boessenkool]

On Fri, Jul 05, 2019 at 05:26:24PM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > Maybe there should be some way of indicating what iterator you want if
> > none is mentioned?  For the whole pattern, or some global priority scheme
> > even?  Changes like (random example)
> >
> >> -(define_insn "*mov<mode>_update2"
> >> +(define_insn "*mov<SFDF:mode>_update2"
> >>    [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
> >>  			  (match_operand:P 2 "reg_or_short_operand" "r,I")))
> >> -	(match_operand:SFDF 3 "gpc_reg_operand" "<Ff>,<Ff>"))
> >> +	(match_operand:SFDF 3 "gpc_reg_operand" "<SFDF:Ff>,<SFDF:Ff>"))
> >>     (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
> >>  	(plus:P (match_dup 1) (match_dup 2)))]
> >>    "TARGET_HARD_FLOAT && TARGET_UPDATE
> >> -   && (!avoiding_indexed_address_p (<MODE>mode)
> >> +   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)
> >
> > do not make the code more readable.  A rule like "Do not use P if any
> > other mode iterator is available" would work for us.
> 
> Yeah, it's a bit ugly, but I think it's better to be explicit even
> for P.  E.g. there are pattern names like:
> 
>     vsx_extract_<P:mode>_<VSX_D:mode>_load
> 
> in which using that rule and dropping the iterator names would silently
> do something unexpected.  (Not a big deal for "*" patterns of course.)

This would be

     vsx_extract_<P:mode>_<mode>_load

in that case.  It *can* still be confused, sure.

> But I think the bigger problem is that attributes tend to grow over
> time, and that something that used to be unambiguous can suddenly
> become ambiguous without anyone noticing.  E.g. an attribute A might
> start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
> pattern.  But then it might be useful to add vector modes to A for some
> unrelated pattern.
> 
> So even if it the order was selectable, it would still be easy to get
> things wrong when you're not thinking about it.  And the series is only
> forcing an iterator to be specified if there's genuine ambiguity.

Yeah.  And there aren't very many places being explicit is needed.  Do
you have some estimate how much that "only disallow if actually ambiguous"
helped?  I expected more changes needed :-)

> E.g. if Ff was specific to floating-point modes, using <Ff> would
> still be OK.

Our Ff unfortunately is not for FP modes only, but for modes that can go
in FP registers :-/


Segher