Iain Sandoe via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Two issues resulted in this PR, which manifests when we force a constant into
> memory in LRA (in PIC code on Darwin). The presence of such forced constants
> is quite dependent on other RTL optimisations, and it is easy for the issue to
> become latent for a specific case.
>
> First, in the Darwin-specific rs6000 backend code, we were not being careful
> enough in rejecting invalid symbolic addresses. Specifically, when generating
> PIC code, we require a SYMBOL_REF to be wrapped in an UNSPEC_MACHOPIC_OFFSET.
>
> Second, LRA was attempting to load a register using an invalid lo_sum address.
>
> The LRA changes are approved in the PR by Vladimir, and the RS6000 changes are
> Darwin-specific (although, of course, any observations are welcome).
>
> Tested on several lo_sum targets and x86_64 all languages except as noted:
> powerpc64-linux (m32/m64) -D
> powerpc64le-linux -D
> powerpc64-aix -Ada -Go -D
> aarch64-linux -Ada -D
> x86_64-linux all langs -D
> powerpc-darwin9 (master and 11.2) -D -Go.
>
> pushed to master, thanks,
> Iain
>
> Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
> Co-authored-by: Vladimir Makarov <vmaka...@redhat.com>
>
> PR target/104117
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000.cc (darwin_rs6000_legitimate_lo_sum_const_p):
> Check for UNSPEC_MACHOPIC_OFFSET wrappers on symbolic addresses when
> emitting PIC code.
> (legitimate_lo_sum_address_p): Likewise.
> * lra-constraints.cc (process_address_1): Do not attempt to emit a reg
> load from an invalid lo_sum address.
> ---
> gcc/config/rs6000/rs6000.cc | 38 +++++++++++++++++++++++++++++++++++--
> gcc/lra-constraints.cc | 17 ++---------------
> 2 files changed, 38 insertions(+), 17 deletions(-)
>
> […]
> diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
> index fdff9e0720a..c700c3f4578 100644
> --- a/gcc/lra-constraints.cc
> +++ b/gcc/lra-constraints.cc
> @@ -3625,21 +3625,8 @@ process_address_1 (int nop, bool check_only_p,
> *ad.inner = gen_rtx_LO_SUM (Pmode, new_reg, addr);
> if (!valid_address_p (op, &ad, cn))
> {
> - /* Try to put lo_sum into register. */
> - insn = emit_insn (gen_rtx_SET
> - (new_reg,
> - gen_rtx_LO_SUM (Pmode, new_reg,
> addr)));
> - code = recog_memoized (insn);
> - if (code >= 0)
> - {
> - *ad.inner = new_reg;
> - if (!valid_address_p (op, &ad, cn))
> - {
> - *ad.inner = addr;
> - code = -1;
> - }
> - }
> -
> + *ad.inner = addr; /* Punt. */
> + code = -1;
> }
> }
> if (code < 0)
Could you go into more details about this? Why is it OK to continue
to try:
(lo_sum new_reg addr)
directly as an address (the context at the top of the hunk), but not try
moving the lo_sum into a register? They should be semantically equivalent,
so it seems that if one is wrong, the other would be too.
Thanks,
Richard
