From: Tom de Vries <Tom_deVries@mentor.com>
To: Vladimir Makarov <vmakarov@redhat.com>
Cc: <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH][IRA] Analysis of register usage of functions for usage by IRA.
Date: Fri, 29 Mar 2013 12:54:00 -0000 [thread overview]
Message-ID: <51558EF4.1030106@mentor.com> (raw)
In-Reply-To: <5141E8A1.5010203@redhat.com>
On 14/03/13 16:11, Vladimir Makarov wrote:
> On 03/14/2013 05:34 AM, Tom de Vries wrote:
>> On 13/02/13 23:35, Vladimir Makarov wrote:
>>
> Actually, I am done with it. In general, it is ok. Although I have
> some minors comments:
>
Vladimir,
Thanks for the review.
I split the patch up into 10 patches, to facilitate further review:
...
0001-Add-command-line-option.patch
0002-Add-new-reg-note-REG_CALL_DECL.patch
0003-Add-implicit-parameter-to-find_all_hard_reg_sets.patch
0004-Add-TARGET_FN_OTHER_HARD_REG_USAGE-hook.patch
0005-Implement-TARGET_FN_OTHER_HARD_REG_USAGE-hook-for-ARM.patch
0006-Collect-register-usage-information.patch
0007-Use-collected-register-usage-information.patch
0008-Enable-by-default-at-O2-and-higher.patch
0009-Add-documentation.patch
0010-Add-test-case.patch
...
I'll post these in reply to this email.
> In Changelog, you missed '*" before cgraph.h:
>
> * haifa-sched.c (recompute_todo_spec, check_clobbered_conditions): Add
> new argument to find_all_hard_reg_sets call.
> cgraph.h (struct cgraph_node): Add function_used_regs,
> function_used_regs_initialized and function_used_regs_valid fields.
>
Fixed (in the log of 0006-Collect-register-usage-information.patch).
>
> @@ -3391,6 +3394,7 @@ df_get_call_refs (struct df_collection_r
> }
> }
> else if (TEST_HARD_REG_BIT (regs_invalidated_by_call, i)
>
> I'd remove the test of regs_invalidated_by_call.
>
> + && TEST_HARD_REG_BIT (fn_reg_set_usage, i)
> /* no clobbers for regs that are the result of the call */
> && !TEST_HARD_REG_BIT (defs_generated, i)
>
Fixed (in 0007-Use-collected-register-usage-information.patch).
> +static void
> +collect_fn_hard_reg_usage (void)
> +{
> + rtx insn;
> + int i;
> + struct cgraph_node *node;
> + struct hard_reg_set_container other_usage;
> +
> + if (!flag_use_caller_save)
> + return;
> +
> + node = cgraph_get_node (current_function_decl);
> + gcc_assert (node != NULL);
> +
> + gcc_assert (!node->function_used_regs_initialized);
> + node->function_used_regs_initialized = 1;
> +
> + for (insn = get_insns (); insn != NULL_RTX; insn = next_insn (insn))
> + {
> + HARD_REG_SET insn_used_regs;
> +
> + if (!NONDEBUG_INSN_P (insn))
> + continue;
> +
> + find_all_hard_reg_sets (insn, &insn_used_regs, false);
> +
> + if (CALL_P (insn)
> + && !get_call_reg_set_usage (insn, &insn_used_regs,
> call_used_reg_set))
> + {
> + CLEAR_HARD_REG_SET (node->function_used_regs);
> + return;
> + }
> +
>
> I'd put it before find_all_hard_reg_sets
>
> + IOR_HARD_REG_SET (node->function_used_regs, insn_used_regs);
> + }
> +
>
>
insn_used_regs is set by both find_all_hard_reg_sets, and by
get_call_reg_set_usage. If we move the IOR to before find_all_hard_reg_sets,
we're using an undefined value.
>
> But you can ignore my two last 2 comments.
>
> The patch is ok for me for trunk at stage1. But I think you need a
> formal approval for df-scan.c, arm.c, mips.c, GCC testsuite expect files
> (lib/target-supports.exp and gcc.target/mips/mips.exp) as I am not a
> maintainer of these parts although these changes look ok for me.
>
I'm assuming you've ok'ed patch 1, 2, 3, 4, 6, 8, 9 and the non-df-scan part of 7.
I'll ask other maintainers about the other parts (5, 10 and the df-scan part of 7).
Thanks,
- Tom
next prev parent reply other threads:[~2013-03-29 12:54 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-25 13:05 Tom de Vries
2013-01-25 15:46 ` Vladimir Makarov
2013-02-07 19:12 ` Tom de Vries
2013-02-13 22:35 ` Vladimir Makarov
2013-03-14 9:35 ` Tom de Vries
2013-03-14 15:22 ` Vladimir Makarov
2013-03-29 12:54 ` Tom de Vries [this message]
2013-03-29 13:06 ` [PATCH][09/10] -fuse-caller-save - Add documentation Tom de Vries
2013-03-29 13:06 ` [PATCH][06/10] -fuse-caller-save - Collect register usage information Tom de Vries
2013-03-29 13:06 ` [PATCH][02/10] -fuse-caller-save - Add new reg-note REG_CALL_DECL Tom de Vries
2013-03-29 13:06 ` [PATCH][04/10] -fuse-caller-save - Add TARGET_FN_OTHER_HARD_REG_USAGE hook Tom de Vries
2013-03-29 13:06 ` [PATCH][05/10] -fuse-caller-save - Implement TARGET_FN_OTHER_HARD_REG_USAGE hook for ARM Tom de Vries
2013-03-29 13:06 ` [PATCH][01/10] -fuse-caller-save - Add command line option Tom de Vries
2013-03-29 13:06 ` [PATCH][07/10] -fuse-caller-save - Use collected register usage information Tom de Vries
2013-03-29 13:06 ` [PATCH][10/10] -fuse-caller-save - Add test-case Tom de Vries
2013-03-29 13:06 ` [PATCH][03/10] -fuse-caller-save - Add implicit parameter to find_all_hard_reg_sets Tom de Vries
2013-03-29 13:06 ` [PATCH][08/10] -fuse-caller-save - Enable by default at O2 and higher Tom de Vries
2013-03-30 16:10 ` [PATCH][IRA] Analysis of register usage of functions for usage by IRA Tom de Vries
2014-01-09 14:42 ` Richard Earnshaw
2014-01-09 20:56 ` Tom de Vries
2014-01-09 21:10 ` Andi Kleen
2014-01-10 0:22 ` Tom de Vries
2014-01-10 11:39 ` Richard Earnshaw
2014-01-10 16:44 ` Tom de Vries
2014-01-13 16:16 ` Tom de Vries
2014-01-14 10:00 ` Richard Earnshaw
2013-03-30 17:11 ` [PATCH][01/10] -fuse-caller-save - Add command line option Tom de Vries
2013-03-30 17:11 ` [PATCH][06/10] -fuse-caller-save - Collect register usage information Tom de Vries
2013-03-30 17:11 ` [PATCH][02/10] -fuse-caller-save - Add new reg-note REG_CALL_DECL Tom de Vries
2013-03-30 17:11 ` [PATCH][05/10] -fuse-caller-save - Implement TARGET_FN_OTHER_HARD_REG_USAGE hook for ARM Tom de Vries
2013-12-06 0:54 ` Tom de Vries
2013-12-09 10:03 ` Richard Earnshaw
2013-03-30 17:11 ` [PATCH][04/10] -fuse-caller-save - Add TARGET_FN_OTHER_HARD_REG_USAGE hook Tom de Vries
2013-12-07 15:07 ` [PATCH] -fuse-caller-save - Implement TARGET_FN_OTHER_HARD_REG_USAGE hook for MIPS Tom de Vries
2013-12-25 13:02 ` Tom de Vries
2014-01-09 13:51 ` [PING^2][PATCH] " Tom de Vries
2014-01-09 15:31 ` Richard Sandiford
2014-01-09 23:43 ` Tom de Vries
2014-01-10 8:47 ` Richard Sandiford
2014-01-13 15:04 ` Tom de Vries
2013-03-30 17:11 ` [PATCH][03/10] -fuse-caller-save - Add implicit parameter to find_all_hard_reg_sets Tom de Vries
2013-03-30 17:12 ` [PATCH][10/10] -fuse-caller-save - Add test-case Tom de Vries
2013-04-28 10:57 ` Richard Sandiford
2013-12-06 0:34 ` Tom de Vries
2013-12-06 8:51 ` Richard Sandiford
2013-03-30 17:12 ` [PATCH][07/10] -fuse-caller-save - Use collected register usage information Tom de Vries
2013-12-06 0:56 ` Tom de Vries
2013-12-06 9:11 ` Paolo Bonzini
2013-03-30 17:12 ` [PATCH][09/10] -fuse-caller-save - Add documentation Tom de Vries
2013-03-30 17:12 ` [PATCH][08/10] -fuse-caller-save - Enable by default at O2 and higher Tom de Vries
2013-12-06 0:47 ` [PATCH][IRA] Analysis of register usage of functions for usage by IRA Tom de Vries
2014-01-14 19:36 ` Vladimir Makarov
2014-05-30 9:20 ` Tom de Vries
2014-09-01 16:41 ` Ulrich Weigand
2014-09-03 16:58 ` Tom de Vries
2014-09-03 18:12 ` Ulrich Weigand
2014-09-03 22:24 ` Tom de Vries
2014-09-04 7:37 ` Tom de Vries
2014-09-04 14:55 ` Vladimir Makarov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51558EF4.1030106@mentor.com \
--to=tom_devries@mentor.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=vmakarov@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).