Re: [03/32] Add a function for getting the ABI of a call insn target

[Jeff Law <law at redhat dot com>]

On 9/30/19 10:32 AM, Richard Sandiford wrote:
> Jeff Law <law@redhat.com> writes:
>> On 9/25/19 9:38 AM, Richard Sandiford wrote:
>>> Richard Sandiford <richard.sandiford@arm.com> writes:
>>>> This patch replaces get_call_reg_set_usage with call_insn_abi,
>>>> which returns the ABI of the target of a call insn.  The ABI's
>>>> full_reg_clobbers corresponds to regs_invalidated_by_call,
>>>> whereas many callers instead passed call_used_or_fixed_regs, i.e.:
>>>>
>>>>   (regs_invalidated_by_call | fixed_reg_set)
>>>>
>>>> The patch slavishly preserves the "| fixed_reg_set" for these callers;
>>>> later patches will clean this up.
>>>
>>> On reflection, I think insn_callee_abi would be a better name for the
>>> function than call_insn_abi, since it should make it clearer that the
>>> function returns the ABI of the target function.  In future we could
>>> have expr_callee_abi for CALL_EXPRs.
>>>
>>> Also, after Segher's comments for 10/32, I've used "callee_abi" as
>>> the name of temporary variables, instead of just "abi".
>>>
>>> I've made the same change for later patches (except where I've posted
>>> new versions instead), but it didn't seem worth spamming the lists
>>> with that.
>>>
>>> Tested as before.
>>>
>>> Richard
>>>
>>> PS. Ping for the series :-)
>>>
>>>
>>> 2019-09-25  Richard Sandiford  <richard.sandiford@arm.com>
>>>
>>> gcc/
>>> 	* target.def (insn_callee_abi): New hook.
>>> 	(remove_extra_call_preserved_regs): Delete.
>>> 	* doc/tm.texi.in (TARGET_INSN_CALLEE_ABI): New macro.
>>> 	(TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>>> 	* doc/tm.texi: Regenerate.
>>> 	* targhooks.h (default_remove_extra_call_preserved_regs): Delete.
>>> 	* targhooks.c (default_remove_extra_call_preserved_regs): Delete.
>>> 	* config/aarch64/aarch64.c (aarch64_simd_call_p): Constify the
>>> 	insn argument.
>>> 	(aarch64_remove_extra_call_preserved_regs): Delete.
>>> 	(aarch64_insn_callee_abi): New function.
>>> 	(TARGET_REMOVE_EXTRA_CALL_PRESERVED_REGS): Delete.
>>> 	(TARGET_INSN_CALLEE_ABI): New macro.
>>> 	* rtl.h (get_call_fndecl): Declare.
>>> 	(cgraph_rtl_info): Fix formatting.  Tweak comment for
>>> 	function_used_regs.  Remove function_used_regs_valid.
>>> 	* rtlanal.c (get_call_fndecl): Moved from final.c
>>> 	* function-abi.h (insn_callee_abi): Declare.
>>> 	(target_function_abi_info): Mention insn_callee_abi.
>>> 	* function-abi.cc (fndecl_abi): Handle flag_ipa_ra in a similar
>>> 	way to get_call_reg_set_usage did.
>>> 	(insn_callee_abi): New function.
>>> 	* regs.h (get_call_reg_set_usage): Delete.
>>> 	* final.c: Include function-abi.h.
>>> 	(collect_fn_hard_reg_usage): Add fixed and stack registers to
>>> 	function_used_regs before the main loop rather than afterwards.
>>> 	Use insn_callee_abi instead of get_call_reg_set_usage.  Exit early
>>> 	if function_used_regs ends up not being useful.
>>> 	(get_call_fndecl): Move to rtlanal.c
>>> 	(get_call_cgraph_rtl_info, get_call_reg_set_usage): Delete.
>>> 	* caller-save.c: Include function-abi.h.
>>> 	(setup_save_areas, save_call_clobbered_regs): Use insn_callee_abi
>>> 	instead of get_call_reg_set_usage.
>>> 	* cfgcleanup.c: Include function-abi.h.
>>> 	(old_insns_match_p): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>> 	* cgraph.h (cgraph_node::rtl_info): Take a const_tree instead of
>>> 	a tree.
>>> 	* cgraph.c (cgraph_node::rtl_info): Likewise.  Initialize
>>> 	function_used_regs.
>>> 	* df-scan.c: Include function-abi.h.
>>> 	(df_get_call_refs): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>> 	* ira-lives.c: Include function-abi.h.
>>> 	(process_bb_node_lives): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>> 	* lra-lives.c: Include function-abi.h.
>>> 	(process_bb_lives): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>> 	* postreload.c: Include function-abi.h.
>>> 	(reload_combine): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>> 	* regcprop.c: Include function-abi.h.
>>> 	(copyprop_hardreg_forward_1): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>> 	* resource.c: Include function-abi.h.
>>> 	(mark_set_resources, mark_target_live_regs): Use insn_callee_abi
>>> 	instead of get_call_reg_set_usage.
>>> 	* var-tracking.c: Include function-abi.h.
>>> 	(dataflow_set_clear_at_call): Use insn_callee_abi instead of
>>> 	get_call_reg_set_usage.
>>>
>> OK.  I think that's the whole set, right?
> 
> Yep, all committed now :-)  Thanks again for the reviews.
> 
>> There were a ton of guality issues over the last couple weeks in the
>> tester, but those were somewhat expected due to a prior patchkit.  The
>> affected targets have new baseline results in my tester, so if anything
>> goes nuts as a result of your patches it should be fairly obvious.
> 
> There was certainly an extra guality failure on aarch64-linux-gnu with
> this series, think it was:
> 
> gcc.dg/guality/pr41616-1.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-object
> 
> Will be interesting to see what effect it has on other targets.
Noted.  I'll pass them along rather than ignoring and forcing new baselines.

jeff