This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] S390: Hotpatching fixes.
- From: Jakub Jelinek <jakub at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- Date: Thu, 26 Mar 2015 21:56:30 +0100
- Subject: Re: [PATCH] S390: Hotpatching fixes.
- Authentication-results: sourceware.org; auth=none
- References: <20150305124019 dot GA6266 at linux dot vnet dot ibm dot com> <20150309112221 dot GA4801 at linux dot vnet dot ibm dot com> <20150309121938 dot GA11867 at linux dot vnet dot ibm dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
On Mon, Mar 09, 2015 at 01:19:38PM +0100, Dominik Vogt wrote:
> @@ -11368,6 +11349,7 @@ static void
> s390_reorg (void)
> {
> bool pool_overflow = false;
> + int hw_before, hw_after;
>
> /* Make sure all splits have been performed; splits after
> machine_dependent_reorg might confuse insn length counts. */
> @@ -11503,6 +11485,40 @@ s390_reorg (void)
> if (insn_added_p)
> shorten_branches (get_insns ());
> }
> +
> + s390_function_num_hotpatch_hw (current_function_decl, &hw_before, &hw_after);
> + if (hw_after > 0)
Two minor issues, both for nested functions:
1) the s390_function_num_hotpatch_hw assigns to ints whose addresses are
passed as arguments, even when it later decides to return false and in this
spot you ignore the return value. Which means that hw_after could be
non-zero, even when you should be ignoring it.
So, either you should check above the return value too, or
change s390_function_num_hotpatch_hw so that it stores 0 for the nested
functions before returning false.
2) as s390_function_num_hotpatch_hw is now called twice for the same
function, for nested functions you'll get the warning reported twice too.
Perhaps add some additional argument whether you want the warning or not
and use it in one of the callers and not in the other one?
Plus supposedly add testsuite coverage for that.
Jakub