Hongtao Liu via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hi:
> For inline asm, there could be an operand like (not (mem:)), it's
> not a valid operand for normal memory constraint.
> Bootstrap is ok, regression test is ok for make check
> RUNTESTFLAGS="--target_board='unix{-m32,}'"
>
> gcc/ChangeLog
> PR target/97540
> * ira.c: (ira_setup_alts): Extract memory from operand only
> for special memory constraint.
> * recog.c (asm_operand_ok): Ditto.
> * lra-constraints.c (process_alt_operands): MEM_P is
> required for normal memory constraint.
>
> gcc/testsuite/ChangeLog
> * gcc.target/i386/pr97540.c: New test.
Sorry to stick my oar in, but I think we should reconsider the
bcst_mem_operand approach. It seems like these patches (and the
previous one) are fighting against the principle that operands
cannot be arbitrary expressions.
This kind of thing was attempted long ago (even before my time!)
for SIGN_EXTEND on MIPS. It ended up causing more problems than
it solved and in the end it had to be taken out. I'm worried that
we might end up going through the same cycle again.
Also, this LRA code is extremely performance-sensitive in terms
of compile time: it's often at the top or near the top of the profile.
So adding calls to new functions like extract_mem_from_operand for
a fairly niche case probably isn't a good trade-off.
I think we should instead find a nice(?) syntax for generating separate
patterns for the two bcst_vector_operand alternatives from a single
.md pattern. That would fit the existing model much more closely.
(To be clear, I'm not saying the existing model is perfect.
I just think a change along these lines is more fundamental
than it might look, and would need changes outside the register
allocators to work reliably.)
FWIW, in:
(define_insn "*<plusminus_insn><mode>3"
[(set (match_operand:VI_AVX2 0 "register_operand" "=x,v")
(plusminus:VI_AVX2
(match_operand:VI_AVX2 1 "bcst_vector_operand" "<comm>0,v")
(match_operand:VI_AVX2 2 "bcst_vector_operand" "xBm,vmBr")))]
we can assume that any bcst_mem_operand will be first. Allowing it
as operand 2 (as the constraints do) creates non-canonical RTL.
So this at least is one case in which I think the bcst_mem_operand
version has to be a separate .md construct.
Sorry for not noticing or speaking up earlier. I realise it's
extremely unhelpful to get this kind of comment after you've done
so much work. :-(
Thanks,
Richard
