Message ID | 20240318105200.3555938-1-hongtao.liu@intel.com |
---|---|
State | New |
Headers | show |
Series | i386 [stv]: Handle REG_EH_REGION note [pr111822]. | expand |
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
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 --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));