Hongtao Liu <crazy...@gmail.com> writes:
> On Fri, Oct 30, 2020 at 1:00 AM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> I guess my main objection is that we have a special memory constraint
>> that isn't in fact matching a MEM (at least not directly).  That seems
>> odd and feels like it's going to come back to bite us.
>>
>> From an RTL perspective, the MEM in the bcst_memory_operand isn't that
>> special.  It's really just a normal memory that happens to appear in a
>> VEC_DUPLICATE.
>>
>> With the MIPS thing, the SIGN_EXTEND was around a register rather
>> than a memory, and the register constraints applied to the register
>> as normal.  In other words, the SIGN_EXTEND was not part of the
>> constraint: target-independent code removed the SIGN_EXTEND
>
> Yes, that's because target-independent code can't distinguish whether
> SIGN_EXTEND is part of constraints. More specifically, constrain_operands
>  will swallow the unary operator when matching constraints. Cut from recog.c
> -----
>           /* A unary operator may be accepted by the predicate, but it
>              is irrelevant for matching constraints.  */
>           /* For special_memory_operand, there could be a memory operand 
> inside,
>              and it would cause a mismatch for constraint_satisfied_p.  */
>           if (UNARY_P (op) && op == extract_mem_from_operand (op))
>             op = XEXP (op, 0);
> ------

Yeah, this is a vestige from the old code to support MIPS-style
SIGN_EXTEND constraints.  I can't remember why it wasn't removed.
Maybe noone thought about it, or maybe it's what H-P said about SH.

reload also still has code for this, but it's telling that LRA and IRA don't.

> But a unary operator with memory would never be accepted by the predicate
> unless it's corresponding to special_memory_constraint, because in IRA/LRA,
> CT_MEMORY would never be matched when op is false for MEM_P.

I think my point is kind of the opposite though: in both the MIPS and the
AVX512 cases, we effectively want the constrained operand to be different
from the operand matched by the predicate.  So I think the old way of
getting target-independent code to dig down into unary expressions is
conceptually cleaner.  Even better would be to have .md constructs that
say exactly what the constraints should match for a given predicate,
but that's obviously more work (and probably not for GCC 11).

I realise it's a bit odd to be warning on the one hand that the old
approach seemed to have problems while at the same time advocating for
following something like the old approach, but still. :-)

For example, there's no conceptual reason why a target couldn't support
VEC_DUPLICATEs of registers.  And if it supported VEC_DUPLICATEs of
both registers and memory, we'd want to allow registers to be spilled
to memory in-place, without generating a reload.  To do that, the thing
being constrained has to be the operand of the VEC_DUPLICATE rather than
the VEC_DUPLICATE itself.

So for AVX512 I think we should handle this by getting target-independent
code to look into the VEC_DUPLICATE and make “Br” match a normal (scalar)
memory operand whose mode satisfies VALID_BCST_MODE_P.

I'm experimenting with introducing accessors for getting the
“constraint version” of recog_data.operand, recog_data.operand_loc
and recog_data.operand_mode (hopefully low-overhead).  For now they
just look inside UNARY_P, as before.  Too early to say whether it'll
work though…

Thanks,
Richard

Reply via email to