on 2022/7/7 17:03, Richard Biener wrote:
> On Thu, Jul 7, 2022 at 10:55 AM Kewen.Lin <li...@linux.ibm.com> wrote:
>>
>> Hi,
>>
>> As test case in PR106091 shows, rs6000 specific pass swaps
>> doesn't preserve the reg_note REG_EH_REGION when replacing
>> some load insn at the end of basic block, it causes the
>> flow info verification to fail unexpectedly. Since memory
>> reference rtx may trap, this patch is to ensure we copy
>> REG_EH_REGION reg_note while replacing swapped aligned load
>> or store.
>>
>> Bootstrapped and regtested on powerpc64-linux-gnu P7 & P8,
>> and powerpc64le-linux-gnu P9 & P10.
>>
>> Richi, could you help to review this patch from a point view
>> of non-call-exceptions expert?
>
> I think it looks OK but I do wonder if in RTL there's a better
> way to transfer EH info from one stmt to another when you
> are replacing it? On gimple gsi_replace would do, but I
> can't immediately find a proper RTL replacement for your
> emit_insn_before (..., X); remove_insn (X); (plus DF assorted
> things).
>
Thanks for so prompt review! For the question, I'm not sure :(,
when I was drafting this patch, I wondered if there is one function
passing/copying reg_note REG_EH_REGION for this kind of need,
so I went through almost all the places related to REG_EH_REGION,
but nothing desired was found (though I may miss sth.).
BR,
Kewen
> Eric?
>
>>
>> I'm going to install it if it looks good to you. Thanks!
>>
>> -----
>> PR target/106091
>>
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000-p8swap.cc (replace_swapped_aligned_store):
>> Copy
>> REG_EH_REGION when replacing one store insn having it.
>> (replace_swapped_aligned_load): Likewise.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/powerpc/pr106091.c: New test.
>> ---
>> gcc/config/rs6000/rs6000-p8swap.cc | 20 ++++++++++++++++++--
>> gcc/testsuite/gcc.target/powerpc/pr106091.c | 15 +++++++++++++++
>> 2 files changed, 33 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106091.c
>>
>> diff --git a/gcc/config/rs6000/rs6000-p8swap.cc
>> b/gcc/config/rs6000/rs6000-p8swap.cc
>> index 275702fee1b..19fbbfb67dc 100644
>> --- a/gcc/config/rs6000/rs6000-p8swap.cc
>> +++ b/gcc/config/rs6000/rs6000-p8swap.cc
>> @@ -1690,7 +1690,15 @@ replace_swapped_aligned_store (swap_web_entry
>> *insn_entry,
>> gcc_assert ((GET_CODE (new_body) == SET)
>> && MEM_P (SET_DEST (new_body)));
>>
>> - set_block_for_insn (new_insn, BLOCK_FOR_INSN (store_insn));
>> + basic_block bb = BLOCK_FOR_INSN (store_insn);
>> + set_block_for_insn (new_insn, bb);
>> + /* Handle REG_EH_REGION note. */
>> + if (cfun->can_throw_non_call_exceptions && BB_END (bb) == store_insn)
>> + {
>> + rtx note = find_reg_note (store_insn, REG_EH_REGION, NULL_RTX);
>> + if (note)
>> + add_reg_note (new_insn, REG_EH_REGION, XEXP (note, 0));
>> + }
>> df_insn_rescan (new_insn);
>>
>> df_insn_delete (store_insn);
>> @@ -1784,7 +1792,15 @@ replace_swapped_aligned_load (swap_web_entry
>> *insn_entry, rtx swap_insn)
>> gcc_assert ((GET_CODE (new_body) == SET)
>> && MEM_P (SET_SRC (new_body)));
>>
>> - set_block_for_insn (new_insn, BLOCK_FOR_INSN (def_insn));
>> + basic_block bb = BLOCK_FOR_INSN (def_insn);
>> + set_block_for_insn (new_insn, bb);
>> + /* Handle REG_EH_REGION note. */
>> + if (cfun->can_throw_non_call_exceptions && BB_END (bb) == def_insn)
>> + {
>> + rtx note = find_reg_note (def_insn, REG_EH_REGION, NULL_RTX);
>> + if (note)
>> + add_reg_note (new_insn, REG_EH_REGION, XEXP (note, 0));
>> + }
>> df_insn_rescan (new_insn);
>>
>> df_insn_delete (def_insn);
>> diff --git a/gcc/testsuite/gcc.target/powerpc/pr106091.c
>> b/gcc/testsuite/gcc.target/powerpc/pr106091.c
>> new file mode 100644
>> index 00000000000..61ce8cf4733
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/powerpc/pr106091.c
>> @@ -0,0 +1,15 @@
>> +/* { dg-options "-O -fnon-call-exceptions -fno-tree-dce -fno-tree-forwprop
>> -w" } */
>> +
>> +/* Verify there is no ICE. */
>> +
>> +typedef short __attribute__ ((__vector_size__ (64))) V;
>> +V v, w;
>> +
>> +inline V foo (V a, V b);
>> +
>> +V
>> +foo (V a, V b)
>> +{
>> + b &= v < b;
>> + return (V){foo (b, w)[3], (V){}[3]};
>> +}
>> --
>> 2.25.1
