Steve Ellcey <sell...@marvell.com> writes:
> OK, I fixed the issues in your last email. I initially found one
> regression while testing. In lra_create_live_ranges_1 I had removed
> the 'call_p = false' statement but did not replaced it with anything.
> This resulted in no regressions on aarch64 but caused a single
> regression on x86 (gcc.target/i386/pr87759.c). I replaced the
> line with 'call_insn = NULL' and the regression went away so I
> have clean bootstraps and no regressions on aarch64 and x86 now.
Looks good to me bar the parameter description below.
> If this looks good to you can I go ahead and check it in? I know
> we are in Stage 3 now, but my recollection is that patches that were
> initially submitted during Stage 1 could go ahead once approved.
Yeah, like you say, this was originally posted in stage 1 and is the
last patch in the series. Not committing it would leave the work
incomplete in GCC 9. The problem is that we're now in stage 4 rather
than stage 3.
I don't think I'm neutral enough to make the call. Richard, Jakub?
> diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
> index a00ec38..b77b675 100644
> --- a/gcc/lra-lives.c
> +++ b/gcc/lra-lives.c
> @@ -576,25 +576,39 @@ lra_setup_reload_pseudo_preferenced_hard_reg (int regno,
>
> /* Check that REGNO living through calls and setjumps, set up conflict
> regs using LAST_CALL_USED_REG_SET, and clear corresponding bits in
> - PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS. */
> + PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS.
> + CALL_INSN may be the specific call we want to check that REGNO lives
> + through or a call that is guaranteed to clobber REGNO if any call
> + in the current block clobbers REGNO. */
I think it would be more accurate to say:
CALL_INSN is a call that is representative of all calls in the region
described by the PSEUDOS_LIVE_THROUGH_* sets, in terms of the registers
that it preserves and clobbers. */
> +
> static inline void
> check_pseudos_live_through_calls (int regno,
> - HARD_REG_SET last_call_used_reg_set)
> + HARD_REG_SET last_call_used_reg_set,
> + rtx_insn *call_insn)
> {
> int hr;
> + rtx_insn *old_call_insn;
>
> if (! sparseset_bit_p (pseudos_live_through_calls, regno))
> return;
> +
> + gcc_assert (call_insn && CALL_P (call_insn));
> + old_call_insn = lra_reg_info[regno].call_insn;
> + if (!old_call_insn
> + || (targetm.return_call_with_max_clobbers
> + && targetm.return_call_with_max_clobbers (old_call_insn, call_insn)
> + == call_insn))
> + lra_reg_info[regno].call_insn = call_insn;
> +
> sparseset_clear_bit (pseudos_live_through_calls, regno);
> IOR_HARD_REG_SET (lra_reg_info[regno].conflict_hard_regs,
> last_call_used_reg_set);
>
> for (hr = 0; HARD_REGISTER_NUM_P (hr); hr++)
> - if (targetm.hard_regno_call_part_clobbered (hr,
> + if (targetm.hard_regno_call_part_clobbered (call_insn, hr,
> PSEUDO_REGNO_MODE (regno)))
> add_to_hard_reg_set (&lra_reg_info[regno].conflict_hard_regs,
> PSEUDO_REGNO_MODE (regno), hr);
> - lra_reg_info[regno].call_p = true;
> if (! sparseset_bit_p (pseudos_live_through_setjumps, regno))
> return;
> sparseset_clear_bit (pseudos_live_through_setjumps, regno);
BTW, I think we could save some compile time by moving the "for" loop
into the new "if", since otherwise call_insn should have no new
information. But that was true before as well (since we could have
skipped the loop if lra_reg_info[regno].call_p was already true),
so it's really a separate issue.
Thanks,
Richard
