| Message ID | 20240827201139.163536-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ipa: Don't disable function parameter analysis for fat LTO streaming | expand |
On Tue, Aug 27, 2024 at 1:11 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > Update analyze_parms not to disable function parameter analysis for > -ffat-lto-objects. Tested on x86-64, there are no differences in zstd > with "-O2 -flto=auto" -g "vs -O2 -flto=auto -g -ffat-lto-objects". > > PR ipa/116410 > * ipa-modref.cc (analyze_parms): Always analyze function parameter > for LTO streaming. > > Signed-off-by: H.J. Lu <hjl.tools@gmail.com> > --- > gcc/ipa-modref.cc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc > index 59cfe91f987..9275030c254 100644 > --- a/gcc/ipa-modref.cc > +++ b/gcc/ipa-modref.cc > @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > summary->arg_flags.safe_grow_cleared (count, true); > summary->arg_flags[parm_index] = EAF_UNUSED; > } > - else if (summary_lto) > + if (summary_lto) > { > if (parm_index >= summary_lto->arg_flags.length ()) > summary_lto->arg_flags.safe_grow_cleared (count, true); > @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > summary->arg_flags.safe_grow_cleared (count, true); > summary->arg_flags[parm_index] = flags; > } > - else if (summary_lto) > + if (summary_lto) > { > if (parm_index >= summary_lto->arg_flags.length ()) > summary_lto->arg_flags.safe_grow_cleared (count, true); > -- > 2.46.0 > These are oversights in https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=85ebbabd85e03bdc3afc190aeb29250606d18322 https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3350e59f2985469b2472e4d9a6d387337da4519b to have if (summary) ... else if (summary_lto) ^^^^ This disables LTO optimization for -ffat-lto-objects. Is this patch OK for master and backports? Thanks. H.J.
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Tue, Aug 27, 2024 at 1:11 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> >> Update analyze_parms not to disable function parameter analysis for >> -ffat-lto-objects. Tested on x86-64, there are no differences in zstd >> with "-O2 -flto=auto" -g "vs -O2 -flto=auto -g -ffat-lto-objects". >> >> PR ipa/116410 >> * ipa-modref.cc (analyze_parms): Always analyze function parameter >> for LTO streaming. >> >> Signed-off-by: H.J. Lu <hjl.tools@gmail.com> >> --- >> gcc/ipa-modref.cc | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc >> index 59cfe91f987..9275030c254 100644 >> --- a/gcc/ipa-modref.cc >> +++ b/gcc/ipa-modref.cc >> @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, >> summary->arg_flags.safe_grow_cleared (count, true); >> summary->arg_flags[parm_index] = EAF_UNUSED; >> } >> - else if (summary_lto) >> + if (summary_lto) >> { >> if (parm_index >= summary_lto->arg_flags.length ()) >> summary_lto->arg_flags.safe_grow_cleared (count, true); >> @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, >> summary->arg_flags.safe_grow_cleared (count, true); >> summary->arg_flags[parm_index] = flags; >> } >> - else if (summary_lto) >> + if (summary_lto) >> { >> if (parm_index >= summary_lto->arg_flags.length ()) >> summary_lto->arg_flags.safe_grow_cleared (count, true); >> -- >> 2.46.0 >> > > These are oversights in > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=85ebbabd85e03bdc3afc190aeb29250606d18322 > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3350e59f2985469b2472e4d9a6d387337da4519b > > to have > > if (summary) > ... > else if (summary_lto) > ^^^^ This disables LTO optimization for -ffat-lto-objects. FWIW, I tested it on amd64 with no regressions and no issues in CPython, Binutils and a few others. I was a bit worried because of modref issues in the past.
On Mon, Sep 2, 2024 at 4:23 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Tue, Aug 27, 2024 at 1:11 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > Update analyze_parms not to disable function parameter analysis for > > -ffat-lto-objects. Tested on x86-64, there are no differences in zstd > > with "-O2 -flto=auto" -g "vs -O2 -flto=auto -g -ffat-lto-objects". > > > > PR ipa/116410 > > * ipa-modref.cc (analyze_parms): Always analyze function parameter > > for LTO streaming. > > > > Signed-off-by: H.J. Lu <hjl.tools@gmail.com> > > --- > > gcc/ipa-modref.cc | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc > > index 59cfe91f987..9275030c254 100644 > > --- a/gcc/ipa-modref.cc > > +++ b/gcc/ipa-modref.cc > > @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > summary->arg_flags.safe_grow_cleared (count, true); > > summary->arg_flags[parm_index] = EAF_UNUSED; > > } > > - else if (summary_lto) > > + if (summary_lto) > > { > > if (parm_index >= summary_lto->arg_flags.length ()) > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > summary->arg_flags.safe_grow_cleared (count, true); > > summary->arg_flags[parm_index] = flags; > > } > > - else if (summary_lto) > > + if (summary_lto) > > { > > if (parm_index >= summary_lto->arg_flags.length ()) > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > -- > > 2.46.0 > > > > These are oversights in > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=85ebbabd85e03bdc3afc190aeb29250606d18322 > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3350e59f2985469b2472e4d9a6d387337da4519b > > to have > > if (summary) > ... > else if (summary_lto) > ^^^^ This disables LTO optimization for -ffat-lto-objects. > > Is this patch OK for master and backports? OK for master. Please wait with backports though, eventually Honza has comments as well. Thanks, Richard. > Thanks. > > H.J. > > -- > H.J.
> > > > > > PR ipa/116410 > > > * ipa-modref.cc (analyze_parms): Always analyze function parameter > > > for LTO streaming. > > > > > > Signed-off-by: H.J. Lu <hjl.tools@gmail.com> > > > --- > > > gcc/ipa-modref.cc | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc > > > index 59cfe91f987..9275030c254 100644 > > > --- a/gcc/ipa-modref.cc > > > +++ b/gcc/ipa-modref.cc > > > @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > > summary->arg_flags.safe_grow_cleared (count, true); > > > summary->arg_flags[parm_index] = EAF_UNUSED; > > > } > > > - else if (summary_lto) > > > + if (summary_lto) > > > { > > > if (parm_index >= summary_lto->arg_flags.length ()) > > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > > @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > > summary->arg_flags.safe_grow_cleared (count, true); > > > summary->arg_flags[parm_index] = flags; > > > } > > > - else if (summary_lto) > > > + if (summary_lto) > > > { > > > if (parm_index >= summary_lto->arg_flags.length ()) > > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > > -- > > > 2.46.0 > > > > > > > These are oversights in > > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=85ebbabd85e03bdc3afc190aeb29250606d18322 > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3350e59f2985469b2472e4d9a6d387337da4519b > > > > to have > > > > if (summary) > > ... > > else if (summary_lto) > > ^^^^ This disables LTO optimization for -ffat-lto-objects. > > > > Is this patch OK for master and backports? > > OK for master. Please wait with backports though, eventually Honza has comments > as well. It looks good to me. The code was originally written for separate LTO and non-LTO paths (since with LTO we can not collect alias sets that are not stable across LTO streaming). Plan was to eventually merge more of the logic by templates, but that did not happen (yet). I will try to look into cleaning this up bit more after adding the nonsequential attribtue Honza > > Thanks, > Richard. > > > Thanks. > > > > H.J. > > > > -- > > H.J.
On Tue, Sep 3, 2024 at 4:00 AM Jan Hubicka <hubicka@ucw.cz> wrote: > > > > > > > > > PR ipa/116410 > > > > * ipa-modref.cc (analyze_parms): Always analyze function parameter > > > > for LTO streaming. > > > > > > > > Signed-off-by: H.J. Lu <hjl.tools@gmail.com> > > > > --- > > > > gcc/ipa-modref.cc | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc > > > > index 59cfe91f987..9275030c254 100644 > > > > --- a/gcc/ipa-modref.cc > > > > +++ b/gcc/ipa-modref.cc > > > > @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > > > summary->arg_flags.safe_grow_cleared (count, true); > > > > summary->arg_flags[parm_index] = EAF_UNUSED; > > > > } > > > > - else if (summary_lto) > > > > + if (summary_lto) > > > > { > > > > if (parm_index >= summary_lto->arg_flags.length ()) > > > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > > > @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > > > summary->arg_flags.safe_grow_cleared (count, true); > > > > summary->arg_flags[parm_index] = flags; > > > > } > > > > - else if (summary_lto) > > > > + if (summary_lto) > > > > { > > > > if (parm_index >= summary_lto->arg_flags.length ()) > > > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > > > -- > > > > 2.46.0 > > > > > > > > > > These are oversights in > > > > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=85ebbabd85e03bdc3afc190aeb29250606d18322 > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3350e59f2985469b2472e4d9a6d387337da4519b > > > > > > to have > > > > > > if (summary) > > > ... > > > else if (summary_lto) > > > ^^^^ This disables LTO optimization for -ffat-lto-objects. > > > > > > Is this patch OK for master and backports? > > > > OK for master. Please wait with backports though, eventually Honza has comments > > as well. > > It looks good to me. The code was originally written for separate LTO > and non-LTO paths (since with LTO we can not collect alias sets that > are not stable across LTO streaming). Plan was to eventually merge more > of the logic by templates, but that did not happen (yet). I will try to > look into cleaning this up bit more after adding the nonsequential > attribtue > OK for backports?
> On Tue, Sep 3, 2024 at 4:00 AM Jan Hubicka <hubicka@ucw.cz> wrote: > > > > > > > > > > > > PR ipa/116410 > > > > > * ipa-modref.cc (analyze_parms): Always analyze function parameter > > > > > for LTO streaming. > > > > > > > > > > Signed-off-by: H.J. Lu <hjl.tools@gmail.com> > > > > > --- > > > > > gcc/ipa-modref.cc | 4 ++-- > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc > > > > > index 59cfe91f987..9275030c254 100644 > > > > > --- a/gcc/ipa-modref.cc > > > > > +++ b/gcc/ipa-modref.cc > > > > > @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > > > > summary->arg_flags.safe_grow_cleared (count, true); > > > > > summary->arg_flags[parm_index] = EAF_UNUSED; > > > > > } > > > > > - else if (summary_lto) > > > > > + if (summary_lto) > > > > > { > > > > > if (parm_index >= summary_lto->arg_flags.length ()) > > > > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > > > > @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, > > > > > summary->arg_flags.safe_grow_cleared (count, true); > > > > > summary->arg_flags[parm_index] = flags; > > > > > } > > > > > - else if (summary_lto) > > > > > + if (summary_lto) > > > > > { > > > > > if (parm_index >= summary_lto->arg_flags.length ()) > > > > > summary_lto->arg_flags.safe_grow_cleared (count, true); > > > > > -- > > > > > 2.46.0 > > > > > > > > > > > > > These are oversights in > > > > > > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=85ebbabd85e03bdc3afc190aeb29250606d18322 > > > > https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=3350e59f2985469b2472e4d9a6d387337da4519b > > > > > > > > to have > > > > > > > > if (summary) > > > > ... > > > > else if (summary_lto) > > > > ^^^^ This disables LTO optimization for -ffat-lto-objects. > > > > > > > > Is this patch OK for master and backports? > > > > > > OK for master. Please wait with backports though, eventually Honza has comments > > > as well. > > > > It looks good to me. The code was originally written for separate LTO > > and non-LTO paths (since with LTO we can not collect alias sets that > > are not stable across LTO streaming). Plan was to eventually merge more > > of the logic by templates, but that did not happen (yet). I will try to > > look into cleaning this up bit more after adding the nonsequential > > attribtue > > > > OK for backports? OK, Thanks! Honza > > > -- > H.J.
diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc index 59cfe91f987..9275030c254 100644 --- a/gcc/ipa-modref.cc +++ b/gcc/ipa-modref.cc @@ -2975,7 +2975,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, summary->arg_flags.safe_grow_cleared (count, true); summary->arg_flags[parm_index] = EAF_UNUSED; } - else if (summary_lto) + if (summary_lto) { if (parm_index >= summary_lto->arg_flags.length ()) summary_lto->arg_flags.safe_grow_cleared (count, true); @@ -3034,7 +3034,7 @@ analyze_parms (modref_summary *summary, modref_summary_lto *summary_lto, summary->arg_flags.safe_grow_cleared (count, true); summary->arg_flags[parm_index] = flags; } - else if (summary_lto) + if (summary_lto) { if (parm_index >= summary_lto->arg_flags.length ()) summary_lto->arg_flags.safe_grow_cleared (count, true);
Update analyze_parms not to disable function parameter analysis for -ffat-lto-objects. Tested on x86-64, there are no differences in zstd with "-O2 -flto=auto" -g "vs -O2 -flto=auto -g -ffat-lto-objects". PR ipa/116410 * ipa-modref.cc (analyze_parms): Always analyze function parameter for LTO streaming. Signed-off-by: H.J. Lu <hjl.tools@gmail.com> --- gcc/ipa-modref.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)