> On Feb 9, 2022, at 10:20 AM, Richard Sandiford <richard.sandif...@arm.com>
> wrote:
>
> Qing Zhao <qing.z...@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.z...@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.z...@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;
>> +}
