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).
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
