middle-end: updating the reg use in exit block for -fzero-call-used-regs [PR100775]

  Hi, Richard,

Could you please review this patch? This is a fix to the previous -fzero-call-used-regs implementation. 

PR 100775 ( ICE: in df_exit_block_bitmap_verify, at df-scan.c:4164 with -mthumb -fzero-call-used-regs=used)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100775

Although the ICE only happens on arm, but this is a bug in the middle end. So, I think this bug has higher priority, 
Need to be included into gcc12, and also need to be back ported to gcc11. 

In the pass_zero_call_used_regs, when updating dataflow info after adding
the register zeroing sequence in the epilogue of the function, we should
call "df_update_exit_block_uses" to update the register use information in
the exit block to include all the registers that have been zeroed.

The change has been bootstrapped and reg-tested on both x86 and aarch64 (with -enable-checking=yes,rtl,df). 
Since I cannot find an arm machine,  no bootstrap and reg-tested on arm yet.

For the arm failure, I just tested it with the cross build and it has no issue withe the fix.

(One question here:
Previously, I though “df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun))” and a later “df_analyze()” should rescan 
the changed exit block of the function, and update all the df info automatically, it apparently not the case, the register
use info at exit block is not automatically updated, we have to add an explicitly call to “df_update_exit_block_uses”.
I checked the pass_thread_prologue_and_epilogue, looks like it also explicitly calls “df_update_entry_exit_and_calls” 
to update the register use info.
Shall the “df_set_bb_dirty” + “df_analyze” automatically update the reg use info of the dirty block?).

Let me know whether there is any issue with the fix?

Thanks

Qing

===================================

From e1cca5659c85e7c536f5016a2c75c615e65dba75 Mon Sep 17 00:00:00 2001
From: Qing Zhao <qing.zhao@oracle.com>
Date: Fri, 28 Jan 2022 16:29:51 +0000
Subject: [PATCH] middle-end: updating the reg use in exit block for
-fzero-call-used-regs [PR100775]

In the pass_zero_call_used_regs, when updating dataflow info after adding
the register zeroing sequence in the epilogue of the function, we should
call "df_update_exit_block_uses" to update the register use information in
the exit block to include all the registers that have been zeroed.

2022-01-27  Qing Zhao  <qing.zhao@oracle.com>

gcc/ChangeLog:

	* function.cc (gen_call_used_regs_seq): Call
	df_update_exit_block_uses when updating df.

gcc/testsuite/ChangeLog:

	* gcc.target/arm/pr100775.c: New test.
---
gcc/function.cc                         | 1 +
gcc/testsuite/gcc.target/arm/pr100775.c | 8 ++++++++
2 files changed, 9 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/arm/pr100775.c
  

Qing Zhao <qing.zhao@oracle.com> writes:
> Hi, Richard,
>
> Could you please review this patch? This is a fix to the previous -fzero-call-used-regs implementation. 
>
> PR 100775 ( ICE: in df_exit_block_bitmap_verify, at df-scan.c:4164 with -mthumb -fzero-call-used-regs=used)
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100775
>
> Although the ICE only happens on arm, but this is a bug in the middle end. So, I think this bug has higher priority, 
> Need to be included into gcc12, and also need to be back ported to gcc11. 
>
> In the pass_zero_call_used_regs, when updating dataflow info after adding
> the register zeroing sequence in the epilogue of the function, we should
> call "df_update_exit_block_uses" to update the register use information in
> the exit block to include all the registers that have been zeroed.
>
> The change has been bootstrapped and reg-tested on both x86 and aarch64 (with -enable-checking=yes,rtl,df). 
> Since I cannot find an arm machine,  no bootstrap and reg-tested on arm yet.
>
> For the arm failure, I just tested it with the cross build and it has no issue withe the fix.
>
> (One question here:
> Previously, I though “df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun))” and a later “df_analyze()” should rescan 
> the changed exit block of the function, and update all the df info automatically, it apparently not the case, the register
> use info at exit block is not automatically updated, we have to add an explicitly call to “df_update_exit_block_uses”.
> I checked the pass_thread_prologue_and_epilogue, looks like it also explicitly calls “df_update_entry_exit_and_calls” 
> to update the register use info.
> Shall the “df_set_bb_dirty” + “df_analyze” automatically update the reg use info of the dirty block?).

I think the current df behaviour makes sense.  Updating the set of
live-out registers is a specialised operation and I think it's better
to make it explicit.

>
> Let me know whether there is any issue with the fix?
>
> Thanks
>
> Qing
>
> ===================================
>
> From e1cca5659c85e7c536f5016a2c75c615e65dba75 Mon Sep 17 00:00:00 2001
> From: Qing Zhao <qing.zhao@oracle.com>
> Date: Fri, 28 Jan 2022 16:29:51 +0000
> Subject: [PATCH] middle-end: updating the reg use in exit block for
> -fzero-call-used-regs [PR100775]
>
> In the pass_zero_call_used_regs, when updating dataflow info after adding
> the register zeroing sequence in the epilogue of the function, we should
> call "df_update_exit_block_uses" to update the register use information in
> the exit block to include all the registers that have been zeroed.
>
> 2022-01-27  Qing Zhao  <qing.zhao@oracle.com>
>
> gcc/ChangeLog:
>
> 	* function.cc (gen_call_used_regs_seq): Call
> 	df_update_exit_block_uses when updating df.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/arm/pr100775.c: New test.
> ---
> gcc/function.cc                         | 1 +
> gcc/testsuite/gcc.target/arm/pr100775.c | 8 ++++++++
> 2 files changed, 9 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/arm/pr100775.c
>
> diff --git a/gcc/function.cc b/gcc/function.cc
> index e1d2565f8d92..c8a77c9a6246 100644
> --- a/gcc/function.cc
> +++ b/gcc/function.cc
> @@ -5942,6 +5942,7 @@ gen_call_used_regs_seq (rtx_insn *ret, unsigned int zero_regs_type)
>       /* Update the data flow information.  */
>       crtl->must_be_zero_on_return |= zeroed_hardregs;
>       df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun));
> +      df_update_exit_block_uses ();

I think this should replace the df_set_bb_dirty call:
df_update_exit_block_uses will mark the block as dirty where
necessary.

Nit, but the indentation of the new call looks off.

>     }
> }
>
> diff --git a/gcc/testsuite/gcc.target/arm/pr100775.c b/gcc/testsuite/gcc.target/arm/pr100775.c
> new file mode 100644
> index 000000000000..dd2255a95492
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr100775.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */

Please add:

/* { dg-skip-if "" { ! { arm_thumb1_ok || arm_thumb2_ok } } } */

here.

OK with those changes, thanks.  Please wait a week or so before
backporting.

Richard

> +/* { dg-options "-mthumb -fzero-call-used-regs=used" } */
> +
> +int
> +foo (int x)
> +{
> +  return x;
> +}
  

> On Feb 9, 2022, at 10:20 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Qing Zhao <qing.zhao@oracle.com> writes:
>> Hi, Richard,
>> 
>> Could you please review this patch? This is a fix to the previous -fzero-call-used-regs implementation. 
>> 
>> PR 100775 ( ICE: in df_exit_block_bitmap_verify, at df-scan.c:4164 with -mthumb -fzero-call-used-regs=used)
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100775
>> 
>> Although the ICE only happens on arm, but this is a bug in the middle end. So, I think this bug has higher priority, 
>> Need to be included into gcc12, and also need to be back ported to gcc11. 
>> 
>> In the pass_zero_call_used_regs, when updating dataflow info after adding
>> the register zeroing sequence in the epilogue of the function, we should
>> call "df_update_exit_block_uses" to update the register use information in
>> the exit block to include all the registers that have been zeroed.
>> 
>> The change has been bootstrapped and reg-tested on both x86 and aarch64 (with -enable-checking=yes,rtl,df). 
>> Since I cannot find an arm machine,  no bootstrap and reg-tested on arm yet.
>> 
>> For the arm failure, I just tested it with the cross build and it has no issue withe the fix.
>> 
>> (One question here:
>> Previously, I though “df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun))” and a later “df_analyze()” should rescan 
>> the changed exit block of the function, and update all the df info automatically, it apparently not the case, the register
>> use info at exit block is not automatically updated, we have to add an explicitly call to “df_update_exit_block_uses”.
>> I checked the pass_thread_prologue_and_epilogue, looks like it also explicitly calls “df_update_entry_exit_and_calls” 
>> to update the register use info.
>> Shall the “df_set_bb_dirty” + “df_analyze” automatically update the reg use info of the dirty block?).
> 
> I think the current df behaviour makes sense.  Updating the set of
> live-out registers is a specialised operation and I think it's better
> to make it explicit.

Okay. I see.
> 
>> 
>> Let me know whether there is any issue with the fix?
>> 
>> Thanks
>> 
>> Qing
>> 
>> ===================================
>> 
>> From e1cca5659c85e7c536f5016a2c75c615e65dba75 Mon Sep 17 00:00:00 2001
>> From: Qing Zhao <qing.zhao@oracle.com>
>> Date: Fri, 28 Jan 2022 16:29:51 +0000
>> Subject: [PATCH] middle-end: updating the reg use in exit block for
>> -fzero-call-used-regs [PR100775]
>> 
>> In the pass_zero_call_used_regs, when updating dataflow info after adding
>> the register zeroing sequence in the epilogue of the function, we should
>> call "df_update_exit_block_uses" to update the register use information in
>> the exit block to include all the registers that have been zeroed.
>> 
>> 2022-01-27  Qing Zhao  <qing.zhao@oracle.com>
>> 
>> gcc/ChangeLog:
>> 
>> 	* function.cc (gen_call_used_regs_seq): Call
>> 	df_update_exit_block_uses when updating df.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.target/arm/pr100775.c: New test.
>> ---
>> gcc/function.cc                         | 1 +
>> gcc/testsuite/gcc.target/arm/pr100775.c | 8 ++++++++
>> 2 files changed, 9 insertions(+)
>> create mode 100644 gcc/testsuite/gcc.target/arm/pr100775.c
>> 
>> diff --git a/gcc/function.cc b/gcc/function.cc
>> index e1d2565f8d92..c8a77c9a6246 100644
>> --- a/gcc/function.cc
>> +++ b/gcc/function.cc
>> @@ -5942,6 +5942,7 @@ gen_call_used_regs_seq (rtx_insn *ret, unsigned int zero_regs_type)
>>      /* Update the data flow information.  */
>>      crtl->must_be_zero_on_return |= zeroed_hardregs;
>>      df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun));
>> +      df_update_exit_block_uses ();
> 
> I think this should replace the df_set_bb_dirty call:
> df_update_exit_block_uses will mark the block as dirty where
> necessary.

Okay, will do that.
> 
> Nit, but the indentation of the new call looks off.
Will fix it.
> 
>>    }
>> }
>> 
>> diff --git a/gcc/testsuite/gcc.target/arm/pr100775.c b/gcc/testsuite/gcc.target/arm/pr100775.c
>> new file mode 100644
>> index 000000000000..dd2255a95492
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/pr100775.c
>> @@ -0,0 +1,8 @@
>> +/* { dg-do compile } */
> 
> Please add:
> 
> /* { dg-skip-if "" { ! { arm_thumb1_ok || arm_thumb2_ok } } } */
Will add it. 
> 
> here.
> 
> OK with those changes, thanks.

Thank you!
>  Please wait a week or so before
> backporting.

Okay.

Qing
> 
> Richard
> 
>> +/* { dg-options "-mthumb -fzero-call-used-regs=used" } */
>> +
>> +int
>> +foo (int x)
>> +{
>> +  return x;
>> +}