Steve Ellcey <sell...@marvell.com> writes:
> /* Implement TARGET_HARD_REGNO_CALL_PART_CLOBBERED. The callee only saves
> the lower 64 bits of a 128-bit register. Tell the compiler the callee
> clobbers the top 64 bits when restoring the bottom 64 bits. */
>
> static bool
> -aarch64_hard_regno_call_part_clobbered (unsigned int regno, machine_mode
> mode)
> +aarch64_hard_regno_call_part_clobbered (rtx_insn *insn, unsigned int regno,
> + machine_mode mode)
> {
> - return FP_REGNUM_P (regno) && maybe_gt (GET_MODE_SIZE (mode), 8);
> + bool simd_p = insn && CALL_P (insn) && aarch64_simd_call_p (insn);
> + return FP_REGNUM_P (regno) && maybe_gt (GET_MODE_SIZE (mode), simd_p ? 16:
> 8);
Should be a space before the ":" (which pushes the line over 80 chars).
> diff --git a/gcc/hooks.h b/gcc/hooks.h
> index 9e4bc29..dc6b2e1 100644
> --- a/gcc/hooks.h
> +++ b/gcc/hooks.h
> @@ -40,7 +40,9 @@ extern bool hook_bool_const_rtx_insn_const_rtx_insn_true
> (const rtx_insn *,
> extern bool hook_bool_mode_uhwi_false (machine_mode,
> unsigned HOST_WIDE_INT);
> extern bool hook_bool_puint64_puint64_true (poly_uint64, poly_uint64);
> -extern bool hook_bool_uint_mode_false (unsigned int, machine_mode);
> +extern bool hook_bool_insn_uint_mode_false (rtx_insn *,
> + unsigned int,
> + machine_mode);
No need to break the line after "rtx_insn *,".
> extern bool hook_bool_uint_mode_true (unsigned int, machine_mode);
> extern bool hook_bool_tree_false (tree);
> extern bool hook_bool_const_tree_false (const_tree);
> diff --git a/gcc/ira-conflicts.c b/gcc/ira-conflicts.c
> index b57468b..b697e57 100644
> --- a/gcc/ira-conflicts.c
> +++ b/gcc/ira-conflicts.c
> @@ -808,7 +808,8 @@ ira_build_conflicts (void)
> regs must conflict with them. */
> for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> if (!TEST_HARD_REG_BIT (call_used_reg_set, regno)
> - && targetm.hard_regno_call_part_clobbered (regno,
> + && targetm.hard_regno_call_part_clobbered (NULL,
> + regno,
> obj_mode))
> {
> SET_HARD_REG_BIT (OBJECT_CONFLICT_HARD_REGS (obj), regno);
No need to break the line after "NULL,".
> diff --git a/gcc/ira-costs.c b/gcc/ira-costs.c
> index e5d8804..7f60712 100644
> --- a/gcc/ira-costs.c
> +++ b/gcc/ira-costs.c
> @@ -2379,7 +2379,8 @@ ira_tune_allocno_costs (void)
> *crossed_calls_clobber_regs)
> && (ira_hard_reg_set_intersection_p (regno, mode,
> call_used_reg_set)
> - || targetm.hard_regno_call_part_clobbered (regno,
> + || targetm.hard_regno_call_part_clobbered (NULL,
> + regno,
> mode)))
> cost += (ALLOCNO_CALL_FREQ (a)
> * (ira_memory_move_cost[mode][rclass][0]
Same here.
> diff --git a/gcc/lra-int.h b/gcc/lra-int.h
> index 9d9e81d..ccc7b00 100644
> --- a/gcc/lra-int.h
> +++ b/gcc/lra-int.h
> @@ -117,6 +117,8 @@ struct lra_reg
> /* This member is set up in lra-lives.c for subsequent
> assignments. */
> lra_copy_t copies;
> + /* Call instruction that may affect this register. */
> + rtx_insn *call_insn;
> };
>
> /* References to the common info about each register. */
If we do this right, I think the new field should be able to replace call_p.
The pseudo crosses a call iff call_insn is nonnull.
I think the field belongs after:
poly_int64 offset;
since it comes under:
/* The following fields are defined only for pseudos. */
rather than:
/* These members are set up in lra-lives.c and updated in
lra-coalesce.c. */
> diff --git a/gcc/lra-lives.c b/gcc/lra-lives.c
> index 7b60691..0b96891 100644
> --- a/gcc/lra-lives.c
> +++ b/gcc/lra-lives.c
> @@ -579,18 +579,26 @@ lra_setup_reload_pseudo_preferenced_hard_reg (int regno,
> PSEUDOS_LIVE_THROUGH_CALLS and PSEUDOS_LIVE_THROUGH_SETJUMPS. */
> 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;
>
> + if (call_insn && CALL_P (call_insn) &&
> targetm.return_call_with_max_clobbers)
> + lra_reg_info[regno].call_insn =
> + targetm.return_call_with_max_clobbers (call_insn,
> + lra_reg_info[regno].call_insn);
> +
This should happen...
> if (! sparseset_bit_p (pseudos_live_through_calls, regno))
> return;
...here, where we know that regno is live across a call like that
described by call_insn.
call_insn should be nonnull and a CALL_P in that case. We should assert
for that regardless of whether targetm.return_call_with_max_clobbers is
nonnull.
The update should be something like:
rtx_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);
No need to break the line after "call_insn,".
> @@ -658,6 +667,17 @@ process_bb_lives (basic_block bb, int &curr_point, bool
> dead_insn_p)
> if (lra_dump_file != NULL)
> fprintf (lra_dump_file, " BB %d\n", bb->index);
>
> + call_insn = NULL;
> + if (targetm.return_call_with_max_clobbers)
> + {
> + FOR_BB_INSNS_REVERSE_SAFE (bb, curr_insn, next)
> + {
> + if (CALL_P (curr_insn))
> + call_insn = targetm.return_call_with_max_clobbers (curr_insn,
> + call_insn);
> + }
> + }
> +
> /* Scan the code of this basic block, noting which pseudos and hard
> regs are born or die.
>
What I meant about keeping track of the current call during the main
block walk is that we shouldn't have this loop. Instead we should
update call_insn when we see a CALL_P...
> @@ -847,7 +867,8 @@ process_bb_lives (basic_block bb, int &curr_point, bool
> dead_insn_p)
> update_pseudo_point (reg->regno, curr_point, USE_POINT);
> mark_regno_live (reg->regno, reg->biggest_mode);
> check_pseudos_live_through_calls (reg->regno,
> - last_call_used_reg_set);
> + last_call_used_reg_set,
> + call_insn);
> }
>
> if (!HARD_REGISTER_NUM_P (reg->regno))
> @@ -896,7 +917,7 @@ process_bb_lives (basic_block bb, int &curr_point, bool
> dead_insn_p)
>
> if (call_p)
> {
> - if (! flag_ipa_ra)
> + if (! flag_ipa_ra && ! targetm.return_call_with_max_clobbers)
> COPY_HARD_REG_SET(last_call_used_reg_set, call_used_reg_set);
> else
> {
...here. In the "if" arm we can set call_insn to the current instruction,
because any call will do. In the "else" arm we should extend this:
bool flush = (! hard_reg_set_empty_p (last_call_used_reg_set)
&& ! hard_reg_set_equal_p (last_call_used_reg_set,
this_call_used_reg_set))
so that we also flush when the old call insn part-clobbers a
different set of registers.
The idea is that we're trying to do something similar to -fipa-ra:
the registers clobbered by a call depend on the call_insn. -fipa-ra
does that by dividing the block into regions with the same call
behaviour. Then:
- pseudos_live_through_calls says which registers are live across
a call in the current region
- last_call_used_reg_set describes the set of registers that are
clobbered by calls in the current region
A new region starts whenever we find a call instruction that clobbers a
different set of registers from those in last_call_used_reg_set (whether
that's more registers or fewer).
We want to divide the block into regions in a similar way for
return_call_with_max_clobbers. A new region starts whenever the
new call preserves a different set of registers from the original call.
This can be tested by something like:
/* Return true if call instructions CALL1 and CALL2 use ABIs that
preserve the same set of registers. */
static bool
calls_have_same_clobbers_p (rtx_insn *call1, rtx_insn *call2)
{
if (!targetm.return_call_with_max_clobbers)
return false;
return (targetm.return_call_with_max_clobbers (call1, call2) == call1
&& targetm.return_call_with_max_clobbers (call2, call1) == call2);
}
I think it would make sense for return_call_with_max_clobbers to only
handle nonnull isnsn (I probably said otherwise earlier, sorry).
> diff --git a/gcc/reload.c b/gcc/reload.c
> index 6cfd5e2..bff84da 100644
> --- a/gcc/reload.c
> +++ b/gcc/reload.c
> @@ -6912,13 +6912,16 @@ find_equiv_reg (rtx goal, rtx_insn *insn, enum
> reg_class rclass, int other,
> if (regno >= 0 && regno < FIRST_PSEUDO_REGISTER)
> for (i = 0; i < nregs; ++i)
> if (call_used_regs[regno + i]
> - || targetm.hard_regno_call_part_clobbered (regno + i, mode))
> + || targetm.hard_regno_call_part_clobbered (NULL,
> + regno + i,
> + mode))
> return 0;
>
> if (valueno >= 0 && valueno < FIRST_PSEUDO_REGISTER)
> for (i = 0; i < valuenregs; ++i)
> if (call_used_regs[valueno + i]
> - || targetm.hard_regno_call_part_clobbered (valueno + i,
> + || targetm.hard_regno_call_part_clobbered (NULL,
> + valueno + i,
> mode))
> return 0;
> }
No need to split the lines after "NULL,".
> diff --git a/gcc/target.def b/gcc/target.def
> index e8f0f70..839d0b9 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -5766,14 +5766,29 @@ DEFHOOK
> (hard_regno_call_part_clobbered,
> "This hook should return true if @var{regno} is partly call-saved and\n\
> partly call-clobbered, and if a value of mode @var{mode} would be partly\n\
> -clobbered by a call. For example, if the low 32 bits of @var{regno} are\n\
> -preserved across a call but higher bits are clobbered, this hook should\n\
> -return true for a 64-bit mode but false for a 32-bit mode.\n\
> +clobbered by the @var{insn} call. If @var{insn} is NULL then it should\n\
maybe "by call instruction @var{insn}".
Thanks,
Richard
