Vladimir Makarov via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> The following patch solves P1 PR99581
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99581
>
> The patch was successfully tested and bootstrapped on x86-64, ppc64le,
> aarch64.
>
> Is it ok for the trunk?
As I mentioned in bugzilla though, the motivation behind this seems
to be that "o" shouldn't need to check for a valid memory address,
even though "m" and "V" do:
--------------------------------------------------------------------------
(define_memory_constraint "TARGET_MEM_CONSTRAINT"
"Matches any valid memory."
(and (match_code "mem")
(match_test "memory_address_addr_space_p (GET_MODE (op), XEXP (op, 0),
MEM_ADDR_SPACE (op))")))
(define_memory_constraint "o"
"Matches an offsettable memory reference."
(and (match_code "mem")
(match_test "offsettable_nonstrict_memref_p (op)")))
;; "V" matches TARGET_MEM_CONSTRAINTs that are rejected by "o".
;; This means that it is not a memory constraint in the usual sense,
;; since reloading the address into a base register would make the
;; address offsettable.
(define_constraint "V"
"Matches a non-offsettable memory reference."
(and (match_code "mem")
(match_test "memory_address_addr_space_p (GET_MODE (op), XEXP (op, 0),
MEM_ADDR_SPACE (op))")
(not (match_test "offsettable_nonstrict_memref_p (op)"))))
--------------------------------------------------------------------------
If the model behind this is that memory constraints should be able
to assume basic validity (in the targetm.legitimate_address sense)
then it seems like "m" should just be:
--------------------------------------------------------------------------
(define_memory_constraint "TARGET_MEM_CONSTRAINT"
"Matches any valid memory."
(match_code "mem"))
--------------------------------------------------------------------------
But it seems to be long-established precedent that "m" isn't just that.
So I'm reluctant to go down this path if there's no clear reason why
"m" has to have the check but "o" doesn't. I think we're also opening
the possiblity that we'll need a define_special_relaxed_memory_constraint
in future.
The reason why my LRA patch seemed OK to me was that we'd already done
the same thing for address constraints:
/* Target hooks sometimes don't treat extra-constraint addresses as
legitimate address_operands, so handle them specially. */
if (insn_extra_address_constraint (cn)
&& satisfies_address_constraint_p (&ad, cn))
return change_p;
which ultimately came from:
https://gcc.gnu.org/g:2c62cbaaf13c78f10657e91efdb8352dc8898b0d
and so is also long-standing precedent as this point.
So I think we should also have a model for why normal address constraints
can do this but normal memory constraints can't.
I'm not trying to reject the patch as such. I just think we need to
have a clearer picture first.
Thanks,
Richard
