| Message ID | CAAe5K+X6nQrqasCKpDG4o5-o65vvP2jtBpkMqiaPSZH6hRGzmA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Sep 27, 2013 at 11:50 AM, Teresa Johnson <tejohnson@google.com> wrote: > David and Rong, > > The following patch will disable -g/-gmlt when instrumenting for LIPO > since they will affect the recorded ggc_memory used in the module > grouping decision. Added -fripa-allow-debug to override this behavior. > > Passes regression tests and simple tests on the new functionality. > > Ok for google/4_8? > > Teresa > > 2013-09-27 Teresa Johnson <tejohnson@google.com> > > * opts.c (finish_options): Suppress -g/-gmlt when we are > building for LIPO instrumention. > * common.opt (fripa-allow-debug): New option. > > Index: opts.c > =================================================================== > --- opts.c (revision 202976) > +++ opts.c (working copy) > @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g > #endif > if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) > error_at (loc, "-fno-fat-lto-objects are supported only with > linker plugin."); > -} > + } Unrelated format change? Otherwise looks ok. thanks, David > if ((opts->x_flag_lto_partition_balanced != 0) + > (opts->x_flag_lto_partition_1to1 != 0) > + (opts->x_flag_lto_partition_none != 0) >= 1) > { > @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g > /* Turn on -ffunction-sections when -freorder-functions=* is used. */ > if (opts->x_flag_reorder_functions > 1) > opts->x_flag_function_sections = 1; > + > + /* LIPO module grouping depends on the memory consumed by the profile-gen > + parsing phase, recorded in a per-module ggc_memory field of the module > + info struct. This will be higher when debug generation is on via > + -g/-gmlt, which causes the FE to generate debug structures that will > + increase the ggc_total_memory. This could in theory cause the module > + groups to be slightly more conservative. Disable -g/-gmlt under > + -fripa -fprofile-generate, but provide an option to override this > + in case we actually need to debug an instrumented binary. */ > + if (opts->x_profile_arc_flag > + && opts->x_flag_dyn_ipa > + && opts->x_debug_info_level != DINFO_LEVEL_NONE > + && !opts->x_flag_dyn_ipa_allow_debug) > + { > + inform (loc, > + "Debug generation via -g option disabled under -fripa " > + "-fprofile-generate (use -fripa-allow-debug to override)"); > + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, > + loc); > + } > } > > #define LEFT_COLUMN 27 > Index: common.opt > =================================================================== > --- common.opt (revision 202976) > +++ common.opt (working copy) > @@ -1155,6 +1155,10 @@ fripa > Common Report Var(flag_dyn_ipa) > Perform Dynamic Inter-Procedural Analysis. > > +fripa-allow-debug > +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) > +Allow -g enablement for -fripa -fprofile-generate compiles. > + > fripa-disallow-asm-modules > Common Report Var(flag_ripa_disallow_asm_modules) > Don't import an auxiliary module if it contains asm statements > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
On Fri, Sep 27, 2013 at 12:01 PM, Xinliang David Li <davidxl@google.com> wrote: > On Fri, Sep 27, 2013 at 11:50 AM, Teresa Johnson <tejohnson@google.com> wrote: >> David and Rong, >> >> The following patch will disable -g/-gmlt when instrumenting for LIPO >> since they will affect the recorded ggc_memory used in the module >> grouping decision. Added -fripa-allow-debug to override this behavior. >> >> Passes regression tests and simple tests on the new functionality. >> >> Ok for google/4_8? >> >> Teresa >> >> 2013-09-27 Teresa Johnson <tejohnson@google.com> >> >> * opts.c (finish_options): Suppress -g/-gmlt when we are >> building for LIPO instrumention. >> * common.opt (fripa-allow-debug): New option. >> >> Index: opts.c >> =================================================================== >> --- opts.c (revision 202976) >> +++ opts.c (working copy) >> @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g >> #endif >> if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) >> error_at (loc, "-fno-fat-lto-objects are supported only with >> linker plugin."); >> -} >> + } > > > Unrelated format change? Well, related in the sense that it messed up my editor's auto-indention logic when making the change below. Ok to include the formatting fix if I mention it in the commit log? Teresa > > Otherwise looks ok. > > thanks, > > David > > >> if ((opts->x_flag_lto_partition_balanced != 0) + >> (opts->x_flag_lto_partition_1to1 != 0) >> + (opts->x_flag_lto_partition_none != 0) >= 1) >> { >> @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g >> /* Turn on -ffunction-sections when -freorder-functions=* is used. */ >> if (opts->x_flag_reorder_functions > 1) >> opts->x_flag_function_sections = 1; >> + >> + /* LIPO module grouping depends on the memory consumed by the profile-gen >> + parsing phase, recorded in a per-module ggc_memory field of the module >> + info struct. This will be higher when debug generation is on via >> + -g/-gmlt, which causes the FE to generate debug structures that will >> + increase the ggc_total_memory. This could in theory cause the module >> + groups to be slightly more conservative. Disable -g/-gmlt under >> + -fripa -fprofile-generate, but provide an option to override this >> + in case we actually need to debug an instrumented binary. */ >> + if (opts->x_profile_arc_flag >> + && opts->x_flag_dyn_ipa >> + && opts->x_debug_info_level != DINFO_LEVEL_NONE >> + && !opts->x_flag_dyn_ipa_allow_debug) >> + { >> + inform (loc, >> + "Debug generation via -g option disabled under -fripa " >> + "-fprofile-generate (use -fripa-allow-debug to override)"); >> + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, >> + loc); >> + } >> } >> >> #define LEFT_COLUMN 27 >> Index: common.opt >> =================================================================== >> --- common.opt (revision 202976) >> +++ common.opt (working copy) >> @@ -1155,6 +1155,10 @@ fripa >> Common Report Var(flag_dyn_ipa) >> Perform Dynamic Inter-Procedural Analysis. >> >> +fripa-allow-debug >> +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) >> +Allow -g enablement for -fripa -fprofile-generate compiles. >> + >> fripa-disallow-asm-modules >> Common Report Var(flag_ripa_disallow_asm_modules) >> Don't import an auxiliary module if it contains asm statements >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
ok. David On Fri, Sep 27, 2013 at 1:03 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Fri, Sep 27, 2013 at 12:01 PM, Xinliang David Li <davidxl@google.com> wrote: >> On Fri, Sep 27, 2013 at 11:50 AM, Teresa Johnson <tejohnson@google.com> wrote: >>> David and Rong, >>> >>> The following patch will disable -g/-gmlt when instrumenting for LIPO >>> since they will affect the recorded ggc_memory used in the module >>> grouping decision. Added -fripa-allow-debug to override this behavior. >>> >>> Passes regression tests and simple tests on the new functionality. >>> >>> Ok for google/4_8? >>> >>> Teresa >>> >>> 2013-09-27 Teresa Johnson <tejohnson@google.com> >>> >>> * opts.c (finish_options): Suppress -g/-gmlt when we are >>> building for LIPO instrumention. >>> * common.opt (fripa-allow-debug): New option. >>> >>> Index: opts.c >>> =================================================================== >>> --- opts.c (revision 202976) >>> +++ opts.c (working copy) >>> @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g >>> #endif >>> if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) >>> error_at (loc, "-fno-fat-lto-objects are supported only with >>> linker plugin."); >>> -} >>> + } >> >> >> Unrelated format change? > > Well, related in the sense that it messed up my editor's > auto-indention logic when making the change below. Ok to include the > formatting fix if I mention it in the commit log? > > Teresa > >> >> Otherwise looks ok. >> >> thanks, >> >> David >> >> >>> if ((opts->x_flag_lto_partition_balanced != 0) + >>> (opts->x_flag_lto_partition_1to1 != 0) >>> + (opts->x_flag_lto_partition_none != 0) >= 1) >>> { >>> @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g >>> /* Turn on -ffunction-sections when -freorder-functions=* is used. */ >>> if (opts->x_flag_reorder_functions > 1) >>> opts->x_flag_function_sections = 1; >>> + >>> + /* LIPO module grouping depends on the memory consumed by the profile-gen >>> + parsing phase, recorded in a per-module ggc_memory field of the module >>> + info struct. This will be higher when debug generation is on via >>> + -g/-gmlt, which causes the FE to generate debug structures that will >>> + increase the ggc_total_memory. This could in theory cause the module >>> + groups to be slightly more conservative. Disable -g/-gmlt under >>> + -fripa -fprofile-generate, but provide an option to override this >>> + in case we actually need to debug an instrumented binary. */ >>> + if (opts->x_profile_arc_flag >>> + && opts->x_flag_dyn_ipa >>> + && opts->x_debug_info_level != DINFO_LEVEL_NONE >>> + && !opts->x_flag_dyn_ipa_allow_debug) >>> + { >>> + inform (loc, >>> + "Debug generation via -g option disabled under -fripa " >>> + "-fprofile-generate (use -fripa-allow-debug to override)"); >>> + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, >>> + loc); >>> + } >>> } >>> >>> #define LEFT_COLUMN 27 >>> Index: common.opt >>> =================================================================== >>> --- common.opt (revision 202976) >>> +++ common.opt (working copy) >>> @@ -1155,6 +1155,10 @@ fripa >>> Common Report Var(flag_dyn_ipa) >>> Perform Dynamic Inter-Procedural Analysis. >>> >>> +fripa-allow-debug >>> +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) >>> +Allow -g enablement for -fripa -fprofile-generate compiles. >>> + >>> fripa-disallow-asm-modules >>> Common Report Var(flag_ripa_disallow_asm_modules) >>> Don't import an auxiliary module if it contains asm statements >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
I don't quite understand here. We use the profile-generate memory consumption to estimate the profile use memory consumption. we still have -g/-gmlt in profile-use compilation. Will this change effectively under estimate the memory use in the use phrase? -Rong On Fri, Sep 27, 2013 at 11:50 AM, Teresa Johnson <tejohnson@google.com> wrote: > David and Rong, > > The following patch will disable -g/-gmlt when instrumenting for LIPO > since they will affect the recorded ggc_memory used in the module > grouping decision. Added -fripa-allow-debug to override this behavior. > > Passes regression tests and simple tests on the new functionality. > > Ok for google/4_8? > > Teresa > > 2013-09-27 Teresa Johnson <tejohnson@google.com> > > * opts.c (finish_options): Suppress -g/-gmlt when we are > building for LIPO instrumention. > * common.opt (fripa-allow-debug): New option. > > Index: opts.c > =================================================================== > --- opts.c (revision 202976) > +++ opts.c (working copy) > @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g > #endif > if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) > error_at (loc, "-fno-fat-lto-objects are supported only with > linker plugin."); > -} > + } > if ((opts->x_flag_lto_partition_balanced != 0) + > (opts->x_flag_lto_partition_1to1 != 0) > + (opts->x_flag_lto_partition_none != 0) >= 1) > { > @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g > /* Turn on -ffunction-sections when -freorder-functions=* is used. */ > if (opts->x_flag_reorder_functions > 1) > opts->x_flag_function_sections = 1; > + > + /* LIPO module grouping depends on the memory consumed by the profile-gen > + parsing phase, recorded in a per-module ggc_memory field of the module > + info struct. This will be higher when debug generation is on via > + -g/-gmlt, which causes the FE to generate debug structures that will > + increase the ggc_total_memory. This could in theory cause the module > + groups to be slightly more conservative. Disable -g/-gmlt under > + -fripa -fprofile-generate, but provide an option to override this > + in case we actually need to debug an instrumented binary. */ > + if (opts->x_profile_arc_flag > + && opts->x_flag_dyn_ipa > + && opts->x_debug_info_level != DINFO_LEVEL_NONE > + && !opts->x_flag_dyn_ipa_allow_debug) > + { > + inform (loc, > + "Debug generation via -g option disabled under -fripa " > + "-fprofile-generate (use -fripa-allow-debug to override)"); > + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, > + loc); > + } > } > > #define LEFT_COLUMN 27 > Index: common.opt > =================================================================== > --- common.opt (revision 202976) > +++ common.opt (working copy) > @@ -1155,6 +1155,10 @@ fripa > Common Report Var(flag_dyn_ipa) > Perform Dynamic Inter-Procedural Analysis. > > +fripa-allow-debug > +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) > +Allow -g enablement for -fripa -fprofile-generate compiles. > + > fripa-disallow-asm-modules > Common Report Var(flag_ripa_disallow_asm_modules) > Don't import an auxiliary module if it contains asm statements > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
The issue is that building the instrumented binary with and without, say, -gmlt, may result in different module grouping. Teresa On Fri, Sep 27, 2013 at 1:18 PM, Rong Xu <xur@google.com> wrote: > I don't quite understand here. We use the profile-generate memory > consumption to estimate the profile use memory consumption. > we still have -g/-gmlt in profile-use compilation. Will this change > effectively under estimate the memory use in the use phrase? > > -Rong > > On Fri, Sep 27, 2013 at 11:50 AM, Teresa Johnson <tejohnson@google.com> wrote: >> David and Rong, >> >> The following patch will disable -g/-gmlt when instrumenting for LIPO >> since they will affect the recorded ggc_memory used in the module >> grouping decision. Added -fripa-allow-debug to override this behavior. >> >> Passes regression tests and simple tests on the new functionality. >> >> Ok for google/4_8? >> >> Teresa >> >> 2013-09-27 Teresa Johnson <tejohnson@google.com> >> >> * opts.c (finish_options): Suppress -g/-gmlt when we are >> building for LIPO instrumention. >> * common.opt (fripa-allow-debug): New option. >> >> Index: opts.c >> =================================================================== >> --- opts.c (revision 202976) >> +++ opts.c (working copy) >> @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g >> #endif >> if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) >> error_at (loc, "-fno-fat-lto-objects are supported only with >> linker plugin."); >> -} >> + } >> if ((opts->x_flag_lto_partition_balanced != 0) + >> (opts->x_flag_lto_partition_1to1 != 0) >> + (opts->x_flag_lto_partition_none != 0) >= 1) >> { >> @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g >> /* Turn on -ffunction-sections when -freorder-functions=* is used. */ >> if (opts->x_flag_reorder_functions > 1) >> opts->x_flag_function_sections = 1; >> + >> + /* LIPO module grouping depends on the memory consumed by the profile-gen >> + parsing phase, recorded in a per-module ggc_memory field of the module >> + info struct. This will be higher when debug generation is on via >> + -g/-gmlt, which causes the FE to generate debug structures that will >> + increase the ggc_total_memory. This could in theory cause the module >> + groups to be slightly more conservative. Disable -g/-gmlt under >> + -fripa -fprofile-generate, but provide an option to override this >> + in case we actually need to debug an instrumented binary. */ >> + if (opts->x_profile_arc_flag >> + && opts->x_flag_dyn_ipa >> + && opts->x_debug_info_level != DINFO_LEVEL_NONE >> + && !opts->x_flag_dyn_ipa_allow_debug) >> + { >> + inform (loc, >> + "Debug generation via -g option disabled under -fripa " >> + "-fprofile-generate (use -fripa-allow-debug to override)"); >> + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, >> + loc); >> + } >> } >> >> #define LEFT_COLUMN 27 >> Index: common.opt >> =================================================================== >> --- common.opt (revision 202976) >> +++ common.opt (working copy) >> @@ -1155,6 +1155,10 @@ fripa >> Common Report Var(flag_dyn_ipa) >> Perform Dynamic Inter-Procedural Analysis. >> >> +fripa-allow-debug >> +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) >> +Allow -g enablement for -fripa -fprofile-generate compiles. >> + >> fripa-disallow-asm-modules >> Common Report Var(flag_ripa_disallow_asm_modules) >> Don't import an auxiliary module if it contains asm statements >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
The key is that grouping results should not dependent on the presence of -g flags. The downside of the patch is that it may slightly underestimate the memory pressure at profile-use time, but that should not be a big problem. David On Fri, Sep 27, 2013 at 1:18 PM, Rong Xu <xur@google.com> wrote: > I don't quite understand here. We use the profile-generate memory > consumption to estimate the profile use memory consumption. > we still have -g/-gmlt in profile-use compilation. Will this change > effectively under estimate the memory use in the use phrase? > > -Rong > > On Fri, Sep 27, 2013 at 11:50 AM, Teresa Johnson <tejohnson@google.com> wrote: >> David and Rong, >> >> The following patch will disable -g/-gmlt when instrumenting for LIPO >> since they will affect the recorded ggc_memory used in the module >> grouping decision. Added -fripa-allow-debug to override this behavior. >> >> Passes regression tests and simple tests on the new functionality. >> >> Ok for google/4_8? >> >> Teresa >> >> 2013-09-27 Teresa Johnson <tejohnson@google.com> >> >> * opts.c (finish_options): Suppress -g/-gmlt when we are >> building for LIPO instrumention. >> * common.opt (fripa-allow-debug): New option. >> >> Index: opts.c >> =================================================================== >> --- opts.c (revision 202976) >> +++ opts.c (working copy) >> @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g >> #endif >> if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) >> error_at (loc, "-fno-fat-lto-objects are supported only with >> linker plugin."); >> -} >> + } >> if ((opts->x_flag_lto_partition_balanced != 0) + >> (opts->x_flag_lto_partition_1to1 != 0) >> + (opts->x_flag_lto_partition_none != 0) >= 1) >> { >> @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g >> /* Turn on -ffunction-sections when -freorder-functions=* is used. */ >> if (opts->x_flag_reorder_functions > 1) >> opts->x_flag_function_sections = 1; >> + >> + /* LIPO module grouping depends on the memory consumed by the profile-gen >> + parsing phase, recorded in a per-module ggc_memory field of the module >> + info struct. This will be higher when debug generation is on via >> + -g/-gmlt, which causes the FE to generate debug structures that will >> + increase the ggc_total_memory. This could in theory cause the module >> + groups to be slightly more conservative. Disable -g/-gmlt under >> + -fripa -fprofile-generate, but provide an option to override this >> + in case we actually need to debug an instrumented binary. */ >> + if (opts->x_profile_arc_flag >> + && opts->x_flag_dyn_ipa >> + && opts->x_debug_info_level != DINFO_LEVEL_NONE >> + && !opts->x_flag_dyn_ipa_allow_debug) >> + { >> + inform (loc, >> + "Debug generation via -g option disabled under -fripa " >> + "-fprofile-generate (use -fripa-allow-debug to override)"); >> + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, >> + loc); >> + } >> } >> >> #define LEFT_COLUMN 27 >> Index: common.opt >> =================================================================== >> --- common.opt (revision 202976) >> +++ common.opt (working copy) >> @@ -1155,6 +1155,10 @@ fripa >> Common Report Var(flag_dyn_ipa) >> Perform Dynamic Inter-Procedural Analysis. >> >> +fripa-allow-debug >> +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) >> +Allow -g enablement for -fripa -fprofile-generate compiles. >> + >> fripa-disallow-asm-modules >> Common Report Var(flag_ripa_disallow_asm_modules) >> Don't import an auxiliary module if it contains asm statements >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
Index: opts.c =================================================================== --- opts.c (revision 202976) +++ opts.c (working copy) @@ -799,7 +799,7 @@ finish_options (struct gcc_options *opts, struct g #endif if (!opts->x_flag_fat_lto_objects && !HAVE_LTO_PLUGIN) error_at (loc, "-fno-fat-lto-objects are supported only with linker plugin."); -} + } if ((opts->x_flag_lto_partition_balanced != 0) + (opts->x_flag_lto_partition_1to1 != 0) + (opts->x_flag_lto_partition_none != 0) >= 1) { @@ -852,6 +852,26 @@ finish_options (struct gcc_options *opts, struct g /* Turn on -ffunction-sections when -freorder-functions=* is used. */ if (opts->x_flag_reorder_functions > 1) opts->x_flag_function_sections = 1; + + /* LIPO module grouping depends on the memory consumed by the profile-gen + parsing phase, recorded in a per-module ggc_memory field of the module + info struct. This will be higher when debug generation is on via + -g/-gmlt, which causes the FE to generate debug structures that will + increase the ggc_total_memory. This could in theory cause the module + groups to be slightly more conservative. Disable -g/-gmlt under + -fripa -fprofile-generate, but provide an option to override this + in case we actually need to debug an instrumented binary. */ + if (opts->x_profile_arc_flag + && opts->x_flag_dyn_ipa + && opts->x_debug_info_level != DINFO_LEVEL_NONE + && !opts->x_flag_dyn_ipa_allow_debug) + { + inform (loc, + "Debug generation via -g option disabled under -fripa " + "-fprofile-generate (use -fripa-allow-debug to override)"); + set_debug_level (NO_DEBUG, DEFAULT_GDB_EXTENSIONS, "0", opts, opts_set, + loc); + } } #define LEFT_COLUMN 27 Index: common.opt =================================================================== --- common.opt (revision 202976) +++ common.opt (working copy) @@ -1155,6 +1155,10 @@ fripa Common Report Var(flag_dyn_ipa) Perform Dynamic Inter-Procedural Analysis. +fripa-allow-debug +Common Report Var(flag_dyn_ipa_allow_debug) Init(0) +Allow -g enablement for -fripa -fprofile-generate compiles. + fripa-disallow-asm-modules Common Report Var(flag_ripa_disallow_asm_modules) Don't import an auxiliary module if it contains asm statements
David and Rong, The following patch will disable -g/-gmlt when instrumenting for LIPO since they will affect the recorded ggc_memory used in the module grouping decision. Added -fripa-allow-debug to override this behavior. Passes regression tests and simple tests on the new functionality. Ok for google/4_8? Teresa 2013-09-27 Teresa Johnson <tejohnson@google.com> * opts.c (finish_options): Suppress -g/-gmlt when we are building for LIPO instrumention. * common.opt (fripa-allow-debug): New option.