Segher Boessenkool <seg...@kernel.crashing.org> writes: > On Fri, Jul 05, 2019 at 05:26:24PM +0100, Richard Sandiford wrote: >> Segher Boessenkool <seg...@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
