Re: [PATCH] S390: Hotpatching fixes.

[Jakub Jelinek <jakub@redhat.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