Message ID | 20160620115515.GA4587@intel.com |
---|---|
State | New |
Headers | show |
On Mon, Jun 20, 2016 at 1:55 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: > TImode register referenced in debug insn can be converted to V1TImode > by scalar to vector optimization. We need to convert a debug insn if > it has a variable in a TImode register. > > Tested on x86-64. OK for trunk? Ilya, does this approach look good to you? Also, does DImode STV conversion need similar handling of debug insns? Uros. > > H.J. > ---- > gcc/ > > PR target/71549 > * config/i386/i386.c (timode_scalar_to_vector_candidate_p): > Return true if debug insn has a variable in TImode register. > (timode_remove_non_convertible_regs): Skip debug insn. > (scalar_chain::convert_insn): Change return type to bool. > (scalar_chain::add_insn): Don't check registers in debug insn. > (dimode_scalar_chain::convert_insn): Change return type to bool > and always return true. > (timode_scalar_chain::convert_insn): Change return type to bool. > Convert V1TImode register to SUBREG TImode in debug insn. Return > false if debug insn isn't converted. Otherwise, return true. > (scalar_chain::convert): Increment converted_insns only if > convert_insn returns true. > > gcc/testsuite/ > > PR target/71549 > * gcc.target/i386/pr71549.c: New test. > --- > gcc/config/i386/i386.c | 58 ++++++++++++++++++++++++++++----- > gcc/testsuite/gcc.target/i386/pr71549.c | 24 ++++++++++++++ > 2 files changed, 74 insertions(+), 8 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr71549.c > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 56a5b9c..e17fc53 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -2845,6 +2845,16 @@ dimode_scalar_to_vector_candidate_p (rtx_insn *insn) > static bool > timode_scalar_to_vector_candidate_p (rtx_insn *insn) > { > + if (DEBUG_INSN_P (insn)) > + { > + /* If a variable is put in a TImode register, which may be > + converted to V1TImode, we need to convert this debug insn. */ > + rtx val = PATTERN (insn); > + return (GET_MODE (val) == TImode > + && GET_CODE (val) == VAR_LOCATION > + && REG_P (PAT_VAR_LOCATION_LOC (val))); > + } > + > rtx def_set = single_set (insn); > > if (!def_set) > @@ -3012,7 +3022,12 @@ timode_remove_non_convertible_regs (bitmap candidates) > > EXECUTE_IF_SET_IN_BITMAP (candidates, 0, id, bi) > { > - rtx def_set = single_set (DF_INSN_UID_GET (id)->insn); > + rtx_insn *insn = DF_INSN_UID_GET (id)->insn; > + /* Debug insn isn't a SET insn. */ > + if (DEBUG_INSN_P (insn)) > + continue; > + > + rtx def_set = single_set (insn); > rtx dest = SET_DEST (def_set); > rtx src = SET_SRC (def_set); > > @@ -3111,7 +3126,7 @@ class scalar_chain > void add_insn (bitmap candidates, unsigned insn_uid); > void analyze_register_chain (bitmap candidates, df_ref ref); > virtual void mark_dual_mode_def (df_ref def) = 0; > - virtual void convert_insn (rtx_insn *insn) = 0; > + virtual bool convert_insn (rtx_insn *insn) = 0; > virtual void convert_registers () = 0; > }; > > @@ -3123,7 +3138,7 @@ class dimode_scalar_chain : public scalar_chain > void mark_dual_mode_def (df_ref def); > rtx replace_with_subreg (rtx x, rtx reg, rtx subreg); > void replace_with_subreg_in_insn (rtx_insn *insn, rtx reg, rtx subreg); > - void convert_insn (rtx_insn *insn); > + bool convert_insn (rtx_insn *insn); > void convert_op (rtx *op, rtx_insn *insn); > void convert_reg (unsigned regno); > void make_vector_copies (unsigned regno); > @@ -3139,7 +3154,7 @@ class timode_scalar_chain : public scalar_chain > > private: > void mark_dual_mode_def (df_ref def); > - void convert_insn (rtx_insn *insn); > + bool convert_insn (rtx_insn *insn); > /* We don't convert registers to difference size. */ > void convert_registers () {} > }; > @@ -3276,6 +3291,10 @@ scalar_chain::add_insn (bitmap candidates, unsigned int insn_uid) > bitmap_set_bit (insns, insn_uid); > > rtx_insn *insn = DF_INSN_UID_GET (insn_uid)->insn; > + /* Debug insn isn't a SET insn. */ > + if (DEBUG_INSN_P (insn)) > + return; > + > rtx def_set = single_set (insn); > if (def_set && REG_P (SET_DEST (def_set)) > && !HARD_REGISTER_P (SET_DEST (def_set))) > @@ -3708,7 +3727,7 @@ dimode_scalar_chain::convert_op (rtx *op, rtx_insn *insn) > > /* Convert INSN to vector mode. */ > > -void > +bool > dimode_scalar_chain::convert_insn (rtx_insn *insn) > { > rtx def_set = single_set (insn); > @@ -3788,13 +3807,34 @@ dimode_scalar_chain::convert_insn (rtx_insn *insn) > INSN_CODE (insn) = -1; > recog_memoized (insn); > df_insn_rescan (insn); > + > + return true; > } > > /* Convert INSN from TImode to V1T1mode. */ > > -void > +bool > timode_scalar_chain::convert_insn (rtx_insn *insn) > { > + if (DEBUG_INSN_P (insn)) > + { > + /* It must be a debug insn with a TImode variable in register. */ > + rtx val = PATTERN (insn); > + gcc_assert (GET_MODE (val) == TImode > + && GET_CODE (val) == VAR_LOCATION); > + rtx loc = PAT_VAR_LOCATION_LOC (val); > + gcc_assert (REG_P (loc)); > + /* Convert V1TImode register, which has been updated by a SET > + insn before, to SUBREG TImode. */ > + if (GET_MODE (loc) == V1TImode) > + { > + PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 0); > + df_insn_rescan (insn); > + return true; > + } > + return false; > + } > + > rtx def_set = single_set (insn); > rtx src = SET_SRC (def_set); > rtx dst = SET_DEST (def_set); > @@ -3858,6 +3898,8 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) > INSN_CODE (insn) = -1; > recog_memoized (insn); > df_insn_rescan (insn); > + > + return true; > } > > void > @@ -3893,8 +3935,8 @@ scalar_chain::convert () > > EXECUTE_IF_SET_IN_BITMAP (insns, 0, id, bi) > { > - convert_insn (DF_INSN_UID_GET (id)->insn); > - converted_insns++; > + if (convert_insn (DF_INSN_UID_GET (id)->insn)) > + converted_insns++; > } > > return converted_insns; > diff --git a/gcc/testsuite/gcc.target/i386/pr71549.c b/gcc/testsuite/gcc.target/i386/pr71549.c > new file mode 100644 > index 0000000..8aac891 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr71549.c > @@ -0,0 +1,24 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -g" } */ > + > +struct S1 > +{ > + int f0; > + int f1; > + int f2; > + int:4; > +} a, b; > + > +void > +fn1 (struct S1 p1) > +{ > + a = p1; > + int c = p1.f0; > +} > + > +int > +main () > +{ > + fn1 (b); > + return 0; > +} > -- > 2.5.5 >
2016-06-20 16:39 GMT+03:00 Uros Bizjak <ubizjak@gmail.com>: > On Mon, Jun 20, 2016 at 1:55 PM, H.J. Lu <hongjiu.lu@intel.com> wrote: >> TImode register referenced in debug insn can be converted to V1TImode >> by scalar to vector optimization. We need to convert a debug insn if >> it has a variable in a TImode register. >> >> Tested on x86-64. OK for trunk? > > Ilya, does this approach look good to you? Also, does DImode STV > conversion need similar handling of debug insns? DImode conversion doesn't change register mode (i.e. never calls PUT_MODE for registers). That keeps debug instructions valid. Overall I don't like the idea of having debug insns in candidates set and in chains. Looks like it is possible to have a chain consisting of a debug insn only which is weird (otherwise I don't see why we may return false in timode_scalar_chain::convert_insn). What about other possible register uses? If debug insns are added to candidates then NONDEBUG_INSN_P check for uses in timode_check_non_convertible_regs becomes invalid, right? If we have (or want) to fix some register uses then it's probably would be better to visit register uses when we convert its mode and make required fix-ups. It seems better to me to not involve debug insns in analysis phase. Also I don't think debug insns should be accounted as optimized instructions because we would get different number of optimized instructions depending on debug info availability which may be inconvenient for dump scans (and it is not a real instruction optimization). Thanks, Ilya > > Uros. > >> >> H.J. >> ---- >> gcc/ >> >> PR target/71549 >> * config/i386/i386.c (timode_scalar_to_vector_candidate_p): >> Return true if debug insn has a variable in TImode register. >> (timode_remove_non_convertible_regs): Skip debug insn. >> (scalar_chain::convert_insn): Change return type to bool. >> (scalar_chain::add_insn): Don't check registers in debug insn. >> (dimode_scalar_chain::convert_insn): Change return type to bool >> and always return true. >> (timode_scalar_chain::convert_insn): Change return type to bool. >> Convert V1TImode register to SUBREG TImode in debug insn. Return >> false if debug insn isn't converted. Otherwise, return true. >> (scalar_chain::convert): Increment converted_insns only if >> convert_insn returns true. >> >> gcc/testsuite/ >> >> PR target/71549 >> * gcc.target/i386/pr71549.c: New test. >> --- >> gcc/config/i386/i386.c | 58 ++++++++++++++++++++++++++++----- >> gcc/testsuite/gcc.target/i386/pr71549.c | 24 ++++++++++++++ >> 2 files changed, 74 insertions(+), 8 deletions(-) >> create mode 100644 gcc/testsuite/gcc.target/i386/pr71549.c >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index 56a5b9c..e17fc53 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -2845,6 +2845,16 @@ dimode_scalar_to_vector_candidate_p (rtx_insn *insn) >> static bool >> timode_scalar_to_vector_candidate_p (rtx_insn *insn) >> { >> + if (DEBUG_INSN_P (insn)) >> + { >> + /* If a variable is put in a TImode register, which may be >> + converted to V1TImode, we need to convert this debug insn. */ >> + rtx val = PATTERN (insn); >> + return (GET_MODE (val) == TImode >> + && GET_CODE (val) == VAR_LOCATION >> + && REG_P (PAT_VAR_LOCATION_LOC (val))); >> + } >> + >> rtx def_set = single_set (insn); >> >> if (!def_set) >> @@ -3012,7 +3022,12 @@ timode_remove_non_convertible_regs (bitmap candidates) >> >> EXECUTE_IF_SET_IN_BITMAP (candidates, 0, id, bi) >> { >> - rtx def_set = single_set (DF_INSN_UID_GET (id)->insn); >> + rtx_insn *insn = DF_INSN_UID_GET (id)->insn; >> + /* Debug insn isn't a SET insn. */ >> + if (DEBUG_INSN_P (insn)) >> + continue; >> + >> + rtx def_set = single_set (insn); >> rtx dest = SET_DEST (def_set); >> rtx src = SET_SRC (def_set); >> >> @@ -3111,7 +3126,7 @@ class scalar_chain >> void add_insn (bitmap candidates, unsigned insn_uid); >> void analyze_register_chain (bitmap candidates, df_ref ref); >> virtual void mark_dual_mode_def (df_ref def) = 0; >> - virtual void convert_insn (rtx_insn *insn) = 0; >> + virtual bool convert_insn (rtx_insn *insn) = 0; >> virtual void convert_registers () = 0; >> }; >> >> @@ -3123,7 +3138,7 @@ class dimode_scalar_chain : public scalar_chain >> void mark_dual_mode_def (df_ref def); >> rtx replace_with_subreg (rtx x, rtx reg, rtx subreg); >> void replace_with_subreg_in_insn (rtx_insn *insn, rtx reg, rtx subreg); >> - void convert_insn (rtx_insn *insn); >> + bool convert_insn (rtx_insn *insn); >> void convert_op (rtx *op, rtx_insn *insn); >> void convert_reg (unsigned regno); >> void make_vector_copies (unsigned regno); >> @@ -3139,7 +3154,7 @@ class timode_scalar_chain : public scalar_chain >> >> private: >> void mark_dual_mode_def (df_ref def); >> - void convert_insn (rtx_insn *insn); >> + bool convert_insn (rtx_insn *insn); >> /* We don't convert registers to difference size. */ >> void convert_registers () {} >> }; >> @@ -3276,6 +3291,10 @@ scalar_chain::add_insn (bitmap candidates, unsigned int insn_uid) >> bitmap_set_bit (insns, insn_uid); >> >> rtx_insn *insn = DF_INSN_UID_GET (insn_uid)->insn; >> + /* Debug insn isn't a SET insn. */ >> + if (DEBUG_INSN_P (insn)) >> + return; >> + >> rtx def_set = single_set (insn); >> if (def_set && REG_P (SET_DEST (def_set)) >> && !HARD_REGISTER_P (SET_DEST (def_set))) >> @@ -3708,7 +3727,7 @@ dimode_scalar_chain::convert_op (rtx *op, rtx_insn *insn) >> >> /* Convert INSN to vector mode. */ >> >> -void >> +bool >> dimode_scalar_chain::convert_insn (rtx_insn *insn) >> { >> rtx def_set = single_set (insn); >> @@ -3788,13 +3807,34 @@ dimode_scalar_chain::convert_insn (rtx_insn *insn) >> INSN_CODE (insn) = -1; >> recog_memoized (insn); >> df_insn_rescan (insn); >> + >> + return true; >> } >> >> /* Convert INSN from TImode to V1T1mode. */ >> >> -void >> +bool >> timode_scalar_chain::convert_insn (rtx_insn *insn) >> { >> + if (DEBUG_INSN_P (insn)) >> + { >> + /* It must be a debug insn with a TImode variable in register. */ >> + rtx val = PATTERN (insn); >> + gcc_assert (GET_MODE (val) == TImode >> + && GET_CODE (val) == VAR_LOCATION); >> + rtx loc = PAT_VAR_LOCATION_LOC (val); >> + gcc_assert (REG_P (loc)); >> + /* Convert V1TImode register, which has been updated by a SET >> + insn before, to SUBREG TImode. */ >> + if (GET_MODE (loc) == V1TImode) >> + { >> + PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 0); >> + df_insn_rescan (insn); >> + return true; >> + } >> + return false; >> + } >> + >> rtx def_set = single_set (insn); >> rtx src = SET_SRC (def_set); >> rtx dst = SET_DEST (def_set); >> @@ -3858,6 +3898,8 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) >> INSN_CODE (insn) = -1; >> recog_memoized (insn); >> df_insn_rescan (insn); >> + >> + return true; >> } >> >> void >> @@ -3893,8 +3935,8 @@ scalar_chain::convert () >> >> EXECUTE_IF_SET_IN_BITMAP (insns, 0, id, bi) >> { >> - convert_insn (DF_INSN_UID_GET (id)->insn); >> - converted_insns++; >> + if (convert_insn (DF_INSN_UID_GET (id)->insn)) >> + converted_insns++; >> } >> >> return converted_insns; >> diff --git a/gcc/testsuite/gcc.target/i386/pr71549.c b/gcc/testsuite/gcc.target/i386/pr71549.c >> new file mode 100644 >> index 0000000..8aac891 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/i386/pr71549.c >> @@ -0,0 +1,24 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -g" } */ >> + >> +struct S1 >> +{ >> + int f0; >> + int f1; >> + int f2; >> + int:4; >> +} a, b; >> + >> +void >> +fn1 (struct S1 p1) >> +{ >> + a = p1; >> + int c = p1.f0; >> +} >> + >> +int >> +main () >> +{ >> + fn1 (b); >> + return 0; >> +} >> -- >> 2.5.5 >>
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 56a5b9c..e17fc53 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2845,6 +2845,16 @@ dimode_scalar_to_vector_candidate_p (rtx_insn *insn) static bool timode_scalar_to_vector_candidate_p (rtx_insn *insn) { + if (DEBUG_INSN_P (insn)) + { + /* If a variable is put in a TImode register, which may be + converted to V1TImode, we need to convert this debug insn. */ + rtx val = PATTERN (insn); + return (GET_MODE (val) == TImode + && GET_CODE (val) == VAR_LOCATION + && REG_P (PAT_VAR_LOCATION_LOC (val))); + } + rtx def_set = single_set (insn); if (!def_set) @@ -3012,7 +3022,12 @@ timode_remove_non_convertible_regs (bitmap candidates) EXECUTE_IF_SET_IN_BITMAP (candidates, 0, id, bi) { - rtx def_set = single_set (DF_INSN_UID_GET (id)->insn); + rtx_insn *insn = DF_INSN_UID_GET (id)->insn; + /* Debug insn isn't a SET insn. */ + if (DEBUG_INSN_P (insn)) + continue; + + rtx def_set = single_set (insn); rtx dest = SET_DEST (def_set); rtx src = SET_SRC (def_set); @@ -3111,7 +3126,7 @@ class scalar_chain void add_insn (bitmap candidates, unsigned insn_uid); void analyze_register_chain (bitmap candidates, df_ref ref); virtual void mark_dual_mode_def (df_ref def) = 0; - virtual void convert_insn (rtx_insn *insn) = 0; + virtual bool convert_insn (rtx_insn *insn) = 0; virtual void convert_registers () = 0; }; @@ -3123,7 +3138,7 @@ class dimode_scalar_chain : public scalar_chain void mark_dual_mode_def (df_ref def); rtx replace_with_subreg (rtx x, rtx reg, rtx subreg); void replace_with_subreg_in_insn (rtx_insn *insn, rtx reg, rtx subreg); - void convert_insn (rtx_insn *insn); + bool convert_insn (rtx_insn *insn); void convert_op (rtx *op, rtx_insn *insn); void convert_reg (unsigned regno); void make_vector_copies (unsigned regno); @@ -3139,7 +3154,7 @@ class timode_scalar_chain : public scalar_chain private: void mark_dual_mode_def (df_ref def); - void convert_insn (rtx_insn *insn); + bool convert_insn (rtx_insn *insn); /* We don't convert registers to difference size. */ void convert_registers () {} }; @@ -3276,6 +3291,10 @@ scalar_chain::add_insn (bitmap candidates, unsigned int insn_uid) bitmap_set_bit (insns, insn_uid); rtx_insn *insn = DF_INSN_UID_GET (insn_uid)->insn; + /* Debug insn isn't a SET insn. */ + if (DEBUG_INSN_P (insn)) + return; + rtx def_set = single_set (insn); if (def_set && REG_P (SET_DEST (def_set)) && !HARD_REGISTER_P (SET_DEST (def_set))) @@ -3708,7 +3727,7 @@ dimode_scalar_chain::convert_op (rtx *op, rtx_insn *insn) /* Convert INSN to vector mode. */ -void +bool dimode_scalar_chain::convert_insn (rtx_insn *insn) { rtx def_set = single_set (insn); @@ -3788,13 +3807,34 @@ dimode_scalar_chain::convert_insn (rtx_insn *insn) INSN_CODE (insn) = -1; recog_memoized (insn); df_insn_rescan (insn); + + return true; } /* Convert INSN from TImode to V1T1mode. */ -void +bool timode_scalar_chain::convert_insn (rtx_insn *insn) { + if (DEBUG_INSN_P (insn)) + { + /* It must be a debug insn with a TImode variable in register. */ + rtx val = PATTERN (insn); + gcc_assert (GET_MODE (val) == TImode + && GET_CODE (val) == VAR_LOCATION); + rtx loc = PAT_VAR_LOCATION_LOC (val); + gcc_assert (REG_P (loc)); + /* Convert V1TImode register, which has been updated by a SET + insn before, to SUBREG TImode. */ + if (GET_MODE (loc) == V1TImode) + { + PAT_VAR_LOCATION_LOC (val) = gen_rtx_SUBREG (TImode, loc, 0); + df_insn_rescan (insn); + return true; + } + return false; + } + rtx def_set = single_set (insn); rtx src = SET_SRC (def_set); rtx dst = SET_DEST (def_set); @@ -3858,6 +3898,8 @@ timode_scalar_chain::convert_insn (rtx_insn *insn) INSN_CODE (insn) = -1; recog_memoized (insn); df_insn_rescan (insn); + + return true; } void @@ -3893,8 +3935,8 @@ scalar_chain::convert () EXECUTE_IF_SET_IN_BITMAP (insns, 0, id, bi) { - convert_insn (DF_INSN_UID_GET (id)->insn); - converted_insns++; + if (convert_insn (DF_INSN_UID_GET (id)->insn)) + converted_insns++; } return converted_insns; diff --git a/gcc/testsuite/gcc.target/i386/pr71549.c b/gcc/testsuite/gcc.target/i386/pr71549.c new file mode 100644 index 0000000..8aac891 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr71549.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -g" } */ + +struct S1 +{ + int f0; + int f1; + int f2; + int:4; +} a, b; + +void +fn1 (struct S1 p1) +{ + a = p1; + int c = p1.f0; +} + +int +main () +{ + fn1 (b); + return 0; +}