| Message ID | 1463183826-28562-1-git-send-email-martin@surround.io |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Fri, 13 May 2016 16:57:06 -0700, Martin Kelly wrote: > -Og (introduced in GCC 4.8) lets you optimize for debugging experience, > which can be useful for when you want optimized code that is nonetheless > debuggable. > > Signed-off-by: Martin Kelly <martin@surround.io> Thanks for submitting this patch. I had never heard of -Og, but it seems like a useful addition. > +config BR2_OPTIMIZE_g > + bool "optimize debugging experience" > + select BR2_HOST_GCC_AT_LEAST_4_8 select? You can't select an option such as BR2_HOST_GCC_AT_LEAST_4_8. How could Buildroot *force* the host machine to have gcc >= 4.8 ? In addition, using BR2_HOST_GCC_AT_LEAST_4_8 is wrong here: what we care about is the version of the *target* compiler, not the version of the host compiler. So this line should instead be: depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 > + help > + Optimize debugging experience. -Og enables optimizations that do not > + interfere with debugging. It should be the optimization level of choice for > + the standard edit-compile-debug cycle, offering a reasonable level of > + optimization while maintaining fast compilation and a good debugging > + experience. If you use multiple -O options, with or without level numbers, > + the last such option is the one that is effective. I believe some of those lines are too long. They should have a maximum length of 72 characters. Would you mind reworking your patch to address those two issues and sending an updated version? Thanks a lot! Thomas
Hi Martin, In addition to Thomas's comments, I'd like to reword the help text a little. On 05/14/16 01:57, Martin Kelly wrote: > -Og (introduced in GCC 4.8) lets you optimize for debugging experience, > which can be useful for when you want optimized code that is nonetheless > debuggable. > > Signed-off-by: Martin Kelly <martin@surround.io> > --- > Config.in | 11 +++++++++++ > package/Makefile.in | 3 +++ > 2 files changed, 14 insertions(+) > > diff --git a/Config.in b/Config.in > index b5cc892..9330de1 100644 > --- a/Config.in > +++ b/Config.in > @@ -510,6 +510,17 @@ config BR2_OPTIMIZE_3 > and also turns on the -finline-functions, -funswitch-loops and > -fgcse-after-reload options. > > +config BR2_OPTIMIZE_g > + bool "optimize debugging experience" bool "optimize for debugging" > + select BR2_HOST_GCC_AT_LEAST_4_8 > + help > + Optimize debugging experience. -Og enables optimizations that do not > + interfere with debugging. It should be the optimization level of choice for > + the standard edit-compile-debug cycle, offering a reasonable level of > + optimization while maintaining fast compilation and a good debugging > + experience. If you use multiple -O options, with or without level numbers, > + the last such option is the one that is effective. The last sentence doesn't fit here. So: Optimize for debugging. This enables optimizations that do not interfere with debugging. It should be the optimisation level of choice for the standard edit-compile-debug cycle, offering a reasonable level of optimization while maintaining fast compilation and a good debugging experience. Regards, Arnout > + > config BR2_OPTIMIZE_S > bool "optimize for size" > help > diff --git a/package/Makefile.in b/package/Makefile.in > index 616bdd0..2d6ff89 100644 > --- a/package/Makefile.in > +++ b/package/Makefile.in > @@ -122,6 +122,9 @@ endif > ifeq ($(BR2_OPTIMIZE_3),y) > TARGET_OPTIMIZATION = -O3 > endif > +ifeq ($(BR2_OPTIMIZE_g),y) > +TARGET_OPTIMIZATION = -Og > +endif > ifeq ($(BR2_OPTIMIZE_S),y) > TARGET_OPTIMIZATION = -Os > endif >
On 05/14/2016 05:25 AM, Thomas Petazzoni wrote: > Hello, > > On Fri, 13 May 2016 16:57:06 -0700, Martin Kelly wrote: >> -Og (introduced in GCC 4.8) lets you optimize for debugging experience, >> which can be useful for when you want optimized code that is nonetheless >> debuggable. >> >> Signed-off-by: Martin Kelly <martin@surround.io> > > Thanks for submitting this patch. I had never heard of -Og, but it > seems like a useful addition. > >> +config BR2_OPTIMIZE_g >> + bool "optimize debugging experience" >> + select BR2_HOST_GCC_AT_LEAST_4_8 > > select? You can't select an option such as BR2_HOST_GCC_AT_LEAST_4_8. > How could Buildroot *force* the host machine to have gcc >= 4.8 ? > > In addition, using BR2_HOST_GCC_AT_LEAST_4_8 is wrong here: what we > care about is the version of the *target* compiler, not the version of > the host compiler. > > So this line should instead be: > > depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 > Thanks, I agree. I wasn't sure whether to use select or depends. In hindsight, I should have checked, but I'll fix up the patch and send a revision. >> + help >> + Optimize debugging experience. -Og enables optimizations that do not >> + interfere with debugging. It should be the optimization level of choice for >> + the standard edit-compile-debug cycle, offering a reasonable level of >> + optimization while maintaining fast compilation and a good debugging >> + experience. If you use multiple -O options, with or without level numbers, >> + the last such option is the one that is effective. > > I believe some of those lines are too long. They should have a maximum > length of 72 characters. > > Would you mind reworking your patch to address those two issues and > sending an updated version? > Got it, will do. I wrapped it to 80 lines.
On 05/14/2016 03:50 PM, Arnout Vandecappelle wrote: > Hi Martin, > > In addition to Thomas's comments, I'd like to reword the help text a > little. > > > > The last sentence doesn't fit here. So: > > Optimize for debugging. This enables optimizations that do not > interfere with debugging. It should be the optimisation level of > choice for the standard edit-compile-debug cycle, offering a > reasonable level of optimization while maintaining fast compilation > and a good debugging experience. > Your wording seems fine. Mine was just copied from the manpage :). I'll revise the patch to use that wording.
On 05/16/2016 04:36 PM, Martin Kelly wrote: > On 05/14/2016 05:25 AM, Thomas Petazzoni wrote: >> Hello, >> >> On Fri, 13 May 2016 16:57:06 -0700, Martin Kelly wrote: >>> -Og (introduced in GCC 4.8) lets you optimize for debugging experience, >>> which can be useful for when you want optimized code that is nonetheless >>> debuggable. >>> >>> Signed-off-by: Martin Kelly <martin@surround.io> >> >> Thanks for submitting this patch. I had never heard of -Og, but it >> seems like a useful addition. >> >>> +config BR2_OPTIMIZE_g >>> + bool "optimize debugging experience" >>> + select BR2_HOST_GCC_AT_LEAST_4_8 >> >> select? You can't select an option such as BR2_HOST_GCC_AT_LEAST_4_8. >> How could Buildroot *force* the host machine to have gcc >= 4.8 ? >> >> In addition, using BR2_HOST_GCC_AT_LEAST_4_8 is wrong here: what we >> care about is the version of the *target* compiler, not the version of >> the host compiler. >> >> So this line should instead be: >> >> depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 >> > > Thanks, I agree. I wasn't sure whether to use select or depends. In > hindsight, I should have checked, but I'll fix up the patch and send a > revision. > I sent a revised patch to the list. Please tell me if you notice anything else to fix up.
diff --git a/Config.in b/Config.in index b5cc892..9330de1 100644 --- a/Config.in +++ b/Config.in @@ -510,6 +510,17 @@ config BR2_OPTIMIZE_3 and also turns on the -finline-functions, -funswitch-loops and -fgcse-after-reload options. +config BR2_OPTIMIZE_g + bool "optimize debugging experience" + select BR2_HOST_GCC_AT_LEAST_4_8 + help + Optimize debugging experience. -Og enables optimizations that do not + interfere with debugging. It should be the optimization level of choice for + the standard edit-compile-debug cycle, offering a reasonable level of + optimization while maintaining fast compilation and a good debugging + experience. If you use multiple -O options, with or without level numbers, + the last such option is the one that is effective. + config BR2_OPTIMIZE_S bool "optimize for size" help diff --git a/package/Makefile.in b/package/Makefile.in index 616bdd0..2d6ff89 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -122,6 +122,9 @@ endif ifeq ($(BR2_OPTIMIZE_3),y) TARGET_OPTIMIZATION = -O3 endif +ifeq ($(BR2_OPTIMIZE_g),y) +TARGET_OPTIMIZATION = -Og +endif ifeq ($(BR2_OPTIMIZE_S),y) TARGET_OPTIMIZATION = -Os endif
-Og (introduced in GCC 4.8) lets you optimize for debugging experience, which can be useful for when you want optimized code that is nonetheless debuggable. Signed-off-by: Martin Kelly <martin@surround.io> --- Config.in | 11 +++++++++++ package/Makefile.in | 3 +++ 2 files changed, 14 insertions(+)