Hi! On Fri, May 21, 2021 at 02:45:18PM -0500, Peter Bergner wrote: > Getting back to this now that trunk is open again... > > On 3/31/21 2:17 PM, Segher Boessenkool wrote: > > On Tue, Mar 30, 2021 at 06:49:29PM -0500, Peter Bergner wrote: > >> The mma_assemble_input_operand predicate does not accept reg+reg indexed > >> addresses which can lead to ICEs. The problem is that the quad_address_p > >> function only accepts reg+offset addresses that are valid for quad word > >> accesses, but not reg+reg addresses which are also valid for quad word > >> accesses when dealing with vector types. The solution used here is to > >> call memory_operand, which uses rs6000_legitimate_address_p to ensure > >> the address is valid. For reg+offset addresses, it uses quad_address_p > >> like > >> before, but for reg+reg addresses, it calls legitimate_indexed_address_p > >> addresses which fixes this specific ICE. > > > > quad_address_p should really only be used for lq/stq (and their prefixed > > forms). Those insns have very different semantics: they are atomic, > > while vector loads and stores are not; and the prefixed form has some > > special semantics (it swaps halves on LE). > > quad_address_p has since day one been used for both lq/stq as well as > for vector loads/stores.
The was "quad_memory_operand" in 2013 already, three years earlier, and it was exclusively for load/store quad insns -- which, as I said, have very different semantics (they are atomic, they have different alignment requirements, they have different addressing modes, they have different semantics in LE -- is there anything the *same* as vector memory operands even?) So it would be much clearer and less error-prone if these different concepts were not artificially put together. And the name already suggests it is only for load/store quad insns, not for vectors! > I think the "quad" here is meant to describe > that the address will be used for a dform quad word memory access > (eg, lq/stq, lxv/stxv and lxvp/stxvp) which use DQ offsets. No. If you look at history, quad_memory_operand is for lq/stq only. > I don't > think quad_address_p cares whether the address is being used for an > atomic access or not. That restriction is done elsewhere it seems. And that is a big part of the problem. Please don't use "quad" things for vectors at all. The code will become simpler, and we might even have a chance of getting it correct! > rs6000: MMA test case ICEs using -O3 [PR99842] > > The mma_assemble_input_operand predicate does not accept reg+reg indexed > addresses which can lead to ICEs. The lxv and lxvp instructions have > indexed forms (lxvx and lxvpx), so the simple solution is to just allow > indexed addresses in the predicate. > diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md > index e21bc745f72..121cbf14810 100644 > --- a/gcc/config/rs6000/predicates.md > +++ b/gcc/config/rs6000/predicates.md > @@ -1172,7 +1172,8 @@ (define_special_predicate "mma_assemble_input_operand" > (match_test "(mode == V16QImode > && (vsx_register_operand (op, mode) > || (MEM_P (op) > - && quad_address_p (XEXP (op, 0), mode, false))))")) > + && (indexed_or_indirect_address (XEXP (op, 0), mode) > + || quad_address_p (XEXP (op, 0), mode, false)))))")) This is okay for trunk and for backports. But please fix it properly on trunk after the backport: don't abuse quad* for vectors anymore! Thanks, Segher
