This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Segher Boessenkool <segher@kernel.crashing.org> writes:
> Hi Richard,
>
> On Fri, Jul 05, 2019 at 04:20:37PM +0100, Richard Sandiford wrote:
>> This patch is part of a series that fixes ambiguous attribute
>> uses in .md files, i.e. cases in which attributes didn't use
>> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
>> have different values depending on the iterator chosen.
>
> I have sometimes wondered which iterator is chosen in ambiguous cases.
> So I finally looked it up, and the doc says
>   The @code{@var{iterator}:} prefix may be omitted, in which case the
>   substitution will be attempted for every iterator expansion.
>
> Well ugh :-)  I always thought there was some rule, but nope.
>
> 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.)

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

> (The patch is fine if the whole series is approved, of course).

Thanks!

Richard

> Thanks,
>
>
> Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]