diff mbox series

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

Message ID 20240318105200.3555938-1-hongtao.liu@intel.com
State New
Headers show
Series i386 [stv]: Handle REG_EH_REGION note [pr111822]. | expand

Commit Message

liuhongt March 18, 2024, 10:52 a.m. UTC 
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(-)

Comments

Uros Bizjak March 18, 2024, 10:58 a.m. UTC | #1 
On Mon, Mar 18, 2024 at 11:52 AM liuhongt <hongtao.liu@intel.com> 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, 0);
-    }
-  else if (CONST_INT_P (*op))
-    {
-      rtx vec_cst;
       rtx tmp = gen_rtx_SUBREG (vmode, gen_reg_rtx (smode), 0);
-
-      /* Prefer all ones vector in case of -1.  */
-      if (constm1_operand (*op, GET_MODE (*op)))
-	vec_cst = CONSTM1_RTX (vmode);
-      else
-	{
-	  unsigned n = GET_MODE_NUNITS (vmode);
-	  rtx *v = XALLOCAVEC (rtx, n);
-	  v[0] = *op;
-	  for (unsigned i = 1; i < n; ++i)
-	    v[i] = const0_rtx;
-	  vec_cst = gen_rtx_CONST_VECTOR (vmode, gen_rtvec_v (n, v));
-	}
+      rtx vec_cst = smode_convert_cst (*op, vmode);
 
       if (!standard_sse_constant_p (vec_cst, vmode))
 	{
@@ -1767,67 +1775,6 @@ timode_scalar_chain::fix_debug_reg_uses (rtx reg)
     }
 }
 
-/* Helper function to convert immediate constant X to V1TImode.  */
-static rtx
-timode_convert_cst (rtx x)
-{
-  /* Prefer all ones vector in case of -1.  */
-  if (constm1_operand (x, TImode))
-    return CONSTM1_RTX (V1TImode);
-
-  rtx *v = XALLOCAVEC (rtx, 1);
-  v[0] = x;
-  return gen_rtx_CONST_VECTOR (V1TImode, gen_rtvec_v (1, v));
-}
-
-/* Convert operand OP in INSN from TImode to V1TImode.  */
-
-void
-timode_scalar_chain::convert_op (rtx *op, rtx_insn *insn)
-{
-  if (GET_MODE (*op) == V1TImode)
-    return;
-
-  *op = copy_rtx_if_shared (*op);
-
-  if (REG_P (*op))
-    *op = gen_rtx_SUBREG (V1TImode, *op, 0);
-  else if (MEM_P (*op))
-    {
-      rtx tmp = gen_reg_rtx (V1TImode);
-      emit_insn_before (gen_rtx_SET (tmp,
-				     gen_gpr_to_xmm_move_src (V1TImode, *op)),
-			insn);
-      *op = tmp;
-
-      if (dump_file)
-	fprintf (dump_file, "  Preloading operand for insn %d into r%d\n",
-		 INSN_UID (insn), REGNO (tmp));
-    }
-  else if (CONST_SCALAR_INT_P (*op))
-    {
-      rtx tmp = gen_reg_rtx (V1TImode);
-      rtx vec_cst = timode_convert_cst (*op);
-
-      if (!standard_sse_constant_p (vec_cst, V1TImode))
-	{
-	  start_sequence ();
-	  vec_cst = validize_mem (force_const_mem (V1TImode, vec_cst));
-	  rtx_insn *seq = get_insns ();
-	  end_sequence ();
-	  emit_insn_before (seq, insn);
-	}
-
-      emit_insn_before (gen_move_insn (tmp, vec_cst), insn);
-      *op = tmp;
-    }
-  else
-    {
-      gcc_assert (SUBREG_P (*op));
-      gcc_assert (GET_MODE (*op) == vmode);
-    }
-}
-
 /* Convert INSN from TImode to V1T1mode.  */
 
 void
@@ -1892,7 +1839,7 @@ timode_scalar_chain::convert_insn (rtx_insn *insn)
 	    }
 	  else
 	    {
-	      src = timode_convert_cst (src);
+	      src = smode_convert_cst (src, V1TImode);
 	      src = validize_mem (force_const_mem (V1TImode, src));
 	      use_move = MEM_P (dst);
 	    }
diff --git a/gcc/config/i386/i386-features.h b/gcc/config/i386/i386-features.h
index b259cf679af..6d60f56daa8 100644
--- a/gcc/config/i386/i386-features.h
+++ b/gcc/config/i386/i386-features.h
@@ -160,6 +160,7 @@ class scalar_chain
   bool build (bitmap candidates, unsigned insn_uid, bitmap disallowed);
   virtual int compute_convert_gain () = 0;
   int convert ();
+  void convert_op (rtx *op, rtx_insn *insn);
 
  protected:
   void add_to_queue (unsigned insn_uid);
@@ -176,7 +177,6 @@ class scalar_chain
   bool analyze_register_chain (bitmap candidates, df_ref ref,
 			       bitmap disallowed);
   virtual void convert_insn (rtx_insn *insn) = 0;
-  virtual void convert_op (rtx *op, rtx_insn *insn) = 0;
 };
 
 class general_scalar_chain : public scalar_chain
@@ -188,7 +188,6 @@ class general_scalar_chain : public scalar_chain
 
  private:
   void convert_insn (rtx_insn *insn) final override;
-  void convert_op (rtx *op, rtx_insn *insn) final override;
   int vector_const_cost (rtx exp);
   rtx convert_rotate (enum rtx_code, rtx op0, rtx op1, rtx_insn *insn);
 };
@@ -202,7 +201,6 @@ class timode_scalar_chain : public scalar_chain
  private:
   void fix_debug_reg_uses (rtx reg);
   void convert_insn (rtx_insn *insn) final override;
-  void convert_op (rtx *op, rtx_insn *insn) final override;
 };
 
 } // anon namespace
Hongtao Liu March 18, 2024, 11:02 a.m. UTC | #2 
On Mon, Mar 18, 2024 at 6:59 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Mon, Mar 18, 2024 at 11:52 AM liuhongt <hongtao.liu@intel.com> 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
> >
diff mbox series

Patch

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