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

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> 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 :-)

No real estimate, but I imagine loads :-)  Having two mode iterators
is pretty common, and forcing a qualifier even when there's no
ambiguity (e.g. using vector queries in a vector x P pattern)
would probably be over the top.

And yeah, I was pleasantly surprised how few places needed changing.
The bug-fix to make-work ratio seemed pretty high in the end (but not
for rs6000).

Richard

>
>> 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