Re: [PATCH] i386 [stv]: Handle REG_EH_REGION note [pr111822].

2024-03-18 Thread Hongtao Liu
On Mon, Mar 18, 2024 at 6:59 PM Uros Bizjak  wrote:
>
> On Mon, Mar 18, 2024 at 11:52 AM liuhongt  wrote:
> >
> > Commit r14-9459-g618e34d56cc38e only handles
> > general_scalar_chain::convert_op. The patch also handles
> > timode_scalar_chain::convert_op to avoid potential similar bug.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> > Ok for trunk and backport to releases/gcc-13 branch?
>
> I have the following patch in testing that merges
> {general,timode}_scalar_chain::convert_op, so in addition to less code
> duplication, it will fix the issue for both chains. WDYT?
It would be better for maintenance, I prefer your patch.
>
> Uros.
>
> >
> > gcc/ChangeLog:
> >
> > PR target/111822
> > * config/i386/i386-features.cc
> > (timode_scalar_chain::convert_op): Handle REG_EH_REGION note.
> > ---
> >  gcc/config/i386/i386-features.cc | 20 +---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/config/i386/i386-features.cc 
> > b/gcc/config/i386/i386-features.cc
> > index c7d7a965901..38f57d96df5 100644
> > --- a/gcc/config/i386/i386-features.cc
> > +++ b/gcc/config/i386/i386-features.cc
> > @@ -1794,12 +1794,26 @@ timode_scalar_chain::convert_op (rtx *op, rtx_insn 
> > *insn)
> >  *op = gen_rtx_SUBREG (V1TImode, *op, 0);
> >else if (MEM_P (*op))
> >  {
> > +  rtx_insn* eh_insn;
> >rtx tmp = gen_reg_rtx (V1TImode);
> > -  emit_insn_before (gen_rtx_SET (tmp,
> > -gen_gpr_to_xmm_move_src (V1TImode, 
> > *op)),
> > -   insn);
> > +  eh_insn
> > +   = emit_insn_before (gen_rtx_SET (tmp,
> > +gen_gpr_to_xmm_move_src (V1TImode,
> > + *op)),
> > +   insn);
> >*op = tmp;
> >
> > +  if (cfun->can_throw_non_call_exceptions)
> > +   {
> > + /* Handle REG_EH_REGION note.  */
> > + rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
> > + if (note)
> > +   {
> > + control_flow_insns.safe_push (eh_insn);
> > + add_reg_note (eh_insn, REG_EH_REGION, XEXP (note, 0));
> > +   }
> > +   }
> > +
> >if (dump_file)
> > fprintf (dump_file, "  Preloading operand for insn %d into r%d\n",
> >  INSN_UID (insn), REGNO (tmp));
> > --
> > 2.31.1
> >



-- 
BR,
Hongtao


Re: [PATCH] i386 [stv]: Handle REG_EH_REGION note [pr111822].

2024-03-18 Thread Uros Bizjak
On Mon, Mar 18, 2024 at 11:52 AM liuhongt  wrote:
>
> Commit r14-9459-g618e34d56cc38e only handles
> general_scalar_chain::convert_op. The patch also handles
> timode_scalar_chain::convert_op to avoid potential similar bug.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk and backport to releases/gcc-13 branch?

I have the following patch in testing that merges
{general,timode}_scalar_chain::convert_op, so in addition to less code
duplication, it will fix the issue for both chains. WDYT?

Uros.

>
> gcc/ChangeLog:
>
> PR target/111822
> * config/i386/i386-features.cc
> (timode_scalar_chain::convert_op): Handle REG_EH_REGION note.
> ---
>  gcc/config/i386/i386-features.cc | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/i386/i386-features.cc 
> b/gcc/config/i386/i386-features.cc
> index c7d7a965901..38f57d96df5 100644
> --- a/gcc/config/i386/i386-features.cc
> +++ b/gcc/config/i386/i386-features.cc
> @@ -1794,12 +1794,26 @@ timode_scalar_chain::convert_op (rtx *op, rtx_insn 
> *insn)
>  *op = gen_rtx_SUBREG (V1TImode, *op, 0);
>else if (MEM_P (*op))
>  {
> +  rtx_insn* eh_insn;
>rtx tmp = gen_reg_rtx (V1TImode);
> -  emit_insn_before (gen_rtx_SET (tmp,
> -gen_gpr_to_xmm_move_src (V1TImode, *op)),
> -   insn);
> +  eh_insn
> +   = emit_insn_before (gen_rtx_SET (tmp,
> +gen_gpr_to_xmm_move_src (V1TImode,
> + *op)),
> +   insn);
>*op = tmp;
>
> +  if (cfun->can_throw_non_call_exceptions)
> +   {
> + /* Handle REG_EH_REGION note.  */
> + rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
> + if (note)
> +   {
> + control_flow_insns.safe_push (eh_insn);
> + add_reg_note (eh_insn, REG_EH_REGION, XEXP (note, 0));
> +   }
> +   }
> +
>if (dump_file)
> fprintf (dump_file, "  Preloading operand for insn %d into r%d\n",
>  INSN_UID (insn), REGNO (tmp));
> --
> 2.31.1
>
diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc
index c7d7a965901..6d7ef28e4b1 100644
--- a/gcc/config/i386/i386-features.cc
+++ b/gcc/config/i386/i386-features.cc
@@ -980,14 +980,36 @@ scalar_chain::convert_reg (rtx_insn *insn, rtx dst, rtx 
src)
 REGNO (src), REGNO (dst), INSN_UID (insn));
 }
 
+
+/* Helper function to convert immediate constant X to vmode.  */
+static rtx
+smode_convert_cst (rtx x, enum machine_mode vmode)
+{
+  /* Prefer all ones vector in case of -1.  */
+  if (constm1_operand (x, GET_MODE (x)))
+return  CONSTM1_RTX (vmode);
+
+  unsigned n = GET_MODE_NUNITS (vmode);
+  rtx *v = XALLOCAVEC (rtx, n);
+  v[0] = x;
+  for (unsigned i = 1; i < n; ++i)
+v[i] = const0_rtx;
+  return gen_rtx_CONST_VECTOR (vmode, gen_rtvec_v (n, v));
+}
+
 /* Convert operand OP in INSN.  We should handle
memory operands and uninitialized registers.
All other register uses are converted during
registers conversion.  */
 
 void
-general_scalar_chain::convert_op (rtx *op, rtx_insn *insn)
+scalar_chain::convert_op (rtx *op, rtx_insn *insn)
 {
+  rtx tmp;
+
+  if (GET_MODE (*op) == V1TImode)
+return;
+
   *op = copy_rtx_if_shared (*op);
 
   if (GET_CODE (*op) == NOT
@@ -998,20 +1020,21 @@ general_scalar_chain::convert_op (rtx *op, rtx_insn 
*insn)
 }
   else if (MEM_P (*op))
 {
-  rtx_insn* eh_insn, *movabs = NULL;
-  rtx tmp = gen_reg_rtx (GET_MODE (*op));
+  rtx_insn *movabs = NULL;
 
   /* Emit MOVABS to load from a 64-bit absolute address to a GPR.  */
   if (!memory_operand (*op, GET_MODE (*op)))
{
- rtx tmp2 = gen_reg_rtx (GET_MODE (*op));
- movabs = emit_insn_before (gen_rtx_SET (tmp2, *op), insn);
+ tmp = gen_reg_rtx (GET_MODE (*op));
+ movabs = emit_insn_before (gen_rtx_SET (tmp, *op), insn);
 
- *op = tmp2;
+ *op = tmp;
}
 
-  eh_insn
-   = emit_insn_before (gen_rtx_SET (gen_rtx_SUBREG (vmode, tmp, 0),
+  tmp = gen_rtx_SUBREG (vmode, gen_reg_rtx (GET_MODE (*op)), 0);
+
+  rtx_insn *eh_insn
+   = emit_insn_before (gen_rtx_SET (copy_rtx (tmp),
 gen_gpr_to_xmm_move_src (vmode, *op)),
insn);
 
@@ -1028,33 +1051,18 @@ general_scalar_chain::convert_op (rtx *op, rtx_insn 
*insn)
}
}
 
-  *op = gen_rtx_SUBREG (vmode, tmp, 0);
+  *op = tmp;
 
   if (dump_file)
fprintf (dump_file, "  Preloading operand for insn %d into r%d\n",
 INSN_UID (insn), REGNO (tmp));
 }
   else if (REG_P (*op))
+*op = gen_rtx_SUBREG (vmode, *op, 0);
+  else if (CONST_SCALAR_INT_P (*op))
 {
-  *op = gen_rtx_SUBREG (vmode, *op, 

[PATCH] i386 [stv]: Handle REG_EH_REGION note [pr111822].

2024-03-18 Thread liuhongt
Commit r14-9459-g618e34d56cc38e only handles
general_scalar_chain::convert_op. The patch also handles
timode_scalar_chain::convert_op to avoid potential similar bug.

Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}.
Ok for trunk and backport to releases/gcc-13 branch?

gcc/ChangeLog:

PR target/111822
* config/i386/i386-features.cc
(timode_scalar_chain::convert_op): Handle REG_EH_REGION note.
---
 gcc/config/i386/i386-features.cc | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/gcc/config/i386/i386-features.cc b/gcc/config/i386/i386-features.cc
index c7d7a965901..38f57d96df5 100644
--- a/gcc/config/i386/i386-features.cc
+++ b/gcc/config/i386/i386-features.cc
@@ -1794,12 +1794,26 @@ timode_scalar_chain::convert_op (rtx *op, rtx_insn 
*insn)
 *op = gen_rtx_SUBREG (V1TImode, *op, 0);
   else if (MEM_P (*op))
 {
+  rtx_insn* eh_insn;
   rtx tmp = gen_reg_rtx (V1TImode);
-  emit_insn_before (gen_rtx_SET (tmp,
-gen_gpr_to_xmm_move_src (V1TImode, *op)),
-   insn);
+  eh_insn
+   = emit_insn_before (gen_rtx_SET (tmp,
+gen_gpr_to_xmm_move_src (V1TImode,
+ *op)),
+   insn);
   *op = tmp;
 
+  if (cfun->can_throw_non_call_exceptions)
+   {
+ /* Handle REG_EH_REGION note.  */
+ rtx note = find_reg_note (insn, REG_EH_REGION, NULL_RTX);
+ if (note)
+   {
+ control_flow_insns.safe_push (eh_insn);
+ add_reg_note (eh_insn, REG_EH_REGION, XEXP (note, 0));
+   }
+   }
+
   if (dump_file)
fprintf (dump_file, "  Preloading operand for insn %d into r%d\n",
 INSN_UID (insn), REGNO (tmp));
-- 
2.31.1