[Buildroot] [PATCH v2] Config.in: add -Og option

[Buildroot] [PATCH v2] Config.in: add -Og option

[Martin Kelly]

-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>
---
Changes based on feedback:
- select --> depends on
- Reworded help text
- Wrapped text to 72 lines
---

 Config.in           | 10 ++++++++++
 package/Makefile.in |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/Config.in b/Config.in
index 9bc8e51..3fe6b7a 100644
--- a/Config.in
+++ b/Config.in
@@ -510,6 +510,16 @@ config BR2_OPTIMIZE_3
 	  and also turns on the -finline-functions, -funswitch-loops and
 	  -fgcse-after-reload options.

+config BR2_OPTIMIZE_g
+	bool "optimize for debugging"
+	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
+	help
+	  Optimize for debugging. This 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.
+
 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
--
2.1.4

[Buildroot] [PATCH v2] Config.in: add -Og option

[Arnout Vandecappelle]

On 05/17/16 01:55, 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>
> ---
> Changes based on feedback:
> - select --> depends on
> - Reworded help text
> - Wrapped text to 72 lines

  Well, actually you didn't: you just copied my text, which I incorrectly 
wrapped at 78 columns instead of 72...

> ---
>
>  Config.in           | 10 ++++++++++
>  package/Makefile.in |  3 +++
>  2 files changed, 13 insertions(+)
>
> diff --git a/Config.in b/Config.in
> index 9bc8e51..3fe6b7a 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -510,6 +510,16 @@ config BR2_OPTIMIZE_3
>  	  and also turns on the -finline-functions, -funswitch-loops and
>  	  -fgcse-after-reload options.
>
> +config BR2_OPTIMIZE_g

  I didn't notice this the first time: config options should be all capitals, 
like BR2_OPTIMIZE_S (for the -Os option).

  Regards,
  Arnout

> +	bool "optimize for debugging"
> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8
> +	help
> +	  Optimize for debugging. This 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.
> +
>  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
> --
> 2.1.4
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] Config.in: add -Og option

[Martin Kelly]

On 05/18/2016 12:52 PM, Arnout Vandecappelle wrote:
> On 05/17/16 01:55, 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>
>> ---
>> Changes based on feedback:
>> - select --> depends on
>> - Reworded help text
>> - Wrapped text to 72 lines
>
>   Well, actually you didn't: you just copied my text, which I
> incorrectly wrapped at 78 columns instead of 72...
>

You may have wrapped to 78, but I rewrapped it to 72 columns, so I think 
the patch I sent is correctly wrapped.

>> ---
>>
>>  Config.in           | 10 ++++++++++
>>  package/Makefile.in |  3 +++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/Config.in b/Config.in
>> index 9bc8e51..3fe6b7a 100644
>> --- a/Config.in
>> +++ b/Config.in
>> @@ -510,6 +510,16 @@ config BR2_OPTIMIZE_3
>>        and also turns on the -finline-functions, -funswitch-loops and
>>        -fgcse-after-reload options.
>>
>> +config BR2_OPTIMIZE_g
>
>   I didn't notice this the first time: config options should be all
> capitals, like BR2_OPTIMIZE_S (for the -Os option).
>

I will change this and send a revised patch. Note that there are 
currently several config options that are not all capital (e.g. 
BR2_STRIP_strip), but perhaps those should change too.

[Buildroot] [PATCH v2] Config.in: add -Og option

[Thomas Petazzoni]

Hello,

On Wed, 18 May 2016 13:06:20 -0700, Martin Kelly wrote:

> I will change this and send a revised patch. Note that there are 
> currently several config options that are not all capital (e.g. 
> BR2_STRIP_strip), but perhaps those should change too.

There are indeed previous options that were introduced a long long long
time ago, when we didn't had such a thorough review process.
Unfortunately, changing them is annoying, as it breaks all the
existing .config files for users. For this reason, we try to not rename
options just for the beauty of it.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2] Config.in: add -Og option

[Arnout Vandecappelle]

On 05/18/16 22:06, Martin Kelly wrote:
> On 05/18/2016 12:52 PM, Arnout Vandecappelle wrote:
>> On 05/17/16 01:55, 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>
>>> ---
>>> Changes based on feedback:
>>> - select --> depends on
>>> - Reworded help text
>>> - Wrapped text to 72 lines
>>
>>   Well, actually you didn't: you just copied my text, which I
>> incorrectly wrapped at 78 columns instead of 72...
>>
>
> You may have wrapped to 78, but I rewrapped it to 72 columns, so I think the
> patch I sent is correctly wrapped.

  By my count, the line 'reasonable level of optimization while maintaining fast 
compilation' is 68 characters long. with the tab + 2 spaces that becomes 78.

>
>>> ---
>>>
>>>  Config.in           | 10 ++++++++++
>>>  package/Makefile.in |  3 +++
>>>  2 files changed, 13 insertions(+)
>>>
>>> diff --git a/Config.in b/Config.in
>>> index 9bc8e51..3fe6b7a 100644
>>> --- a/Config.in
>>> +++ b/Config.in
>>> @@ -510,6 +510,16 @@ config BR2_OPTIMIZE_3
>>>        and also turns on the -finline-functions, -funswitch-loops and
>>>        -fgcse-after-reload options.
>>>
>>> +config BR2_OPTIMIZE_g
>>
>>   I didn't notice this the first time: config options should be all
>> capitals, like BR2_OPTIMIZE_S (for the -Os option).
>>
>
> I will change this and send a revised patch. Note that there are currently
> several config options that are not all capital (e.g. BR2_STRIP_strip), but
> perhaps those should change too.

  Historical accident. It's not important enough to change it.


  Regards,
  Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v2] Config.in: add -Og option

[Martin Kelly]

On 05/18/2016 03:00 PM, Arnout Vandecappelle wrote:
> On 05/18/16 22:06, Martin Kelly wrote:
>> On 05/18/2016 12:52 PM, Arnout Vandecappelle wrote:
>>> On 05/17/16 01:55, 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>
>>>> ---
>>>> Changes based on feedback:
>>>> - select --> depends on
>>>> - Reworded help text
>>>> - Wrapped text to 72 lines
>>>
>>>   Well, actually you didn't: you just copied my text, which I
>>> incorrectly wrapped at 78 columns instead of 72...
>>>
>>
>> You may have wrapped to 78, but I rewrapped it to 72 columns, so I
>> think the
>> patch I sent is correctly wrapped.
>
>   By my count, the line 'reasonable level of optimization while
> maintaining fast compilation' is 68 characters long. with the tab + 2
> spaces that becomes 78.
>

I was working under the assumption that tabs count as 1 character. If 
tabs count as 8 characters instead, then the rest of the file is already 
miswrapped; there are many lines containing a tab, 2 spaces, and 70 
characters after. Under BR_OPTIMIZE_2, the line starting with "the 
performance of the generated code" is an example of that. In addition, 
most text editors seem to count a tab as 1 character when displaying 
width (Vim certainly does).

If the intention is to count a tab as 8 characters, I'd be happy to do 
so, but then we should also rewrap the rest of the file for consistency.

>>
>>>> ---
>>>>
>>>>  Config.in           | 10 ++++++++++
>>>>  package/Makefile.in |  3 +++
>>>>  2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/Config.in b/Config.in
>>>> index 9bc8e51..3fe6b7a 100644
>>>> --- a/Config.in
>>>> +++ b/Config.in
>>>> @@ -510,6 +510,16 @@ config BR2_OPTIMIZE_3
>>>>        and also turns on the -finline-functions, -funswitch-loops and
>>>>        -fgcse-after-reload options.
>>>>
>>>> +config BR2_OPTIMIZE_g
>>>
>>>   I didn't notice this the first time: config options should be all
>>> capitals, like BR2_OPTIMIZE_S (for the -Os option).
>>>
>>
>> I will change this and send a revised patch. Note that there are
>> currently
>> several config options that are not all capital (e.g.
>> BR2_STRIP_strip), but
>> perhaps those should change too.
>
>   Historical accident. It's not important enough to change it.
>

Agreed.

[Buildroot] [PATCH v2] Config.in: add -Og option

[Martin Kelly]

On 05/18/2016 03:13 PM, Martin Kelly wrote:
>
> I was working under the assumption that tabs count as 1 character. If
> tabs count as 8 characters instead, then the rest of the file is already
> miswrapped; there are many lines containing a tab, 2 spaces, and 70
> characters after.

Correction: "there are many lines containing a tab, 2 spaces, and 68 
characters after".

[Buildroot] [PATCH v2] Config.in: add -Og option

[Arnout Vandecappelle]

On 05/19/16 00:13, Martin Kelly wrote:
> On 05/18/2016 03:00 PM, Arnout Vandecappelle wrote:
>> On 05/18/16 22:06, Martin Kelly wrote:
>>> On 05/18/2016 12:52 PM, Arnout Vandecappelle wrote:
>>>> On 05/17/16 01:55, 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>
>>>>> ---
>>>>> Changes based on feedback:
>>>>> - select --> depends on
>>>>> - Reworded help text
>>>>> - Wrapped text to 72 lines
>>>>
>>>>   Well, actually you didn't: you just copied my text, which I
>>>> incorrectly wrapped at 78 columns instead of 72...
>>>>
>>>
>>> You may have wrapped to 78, but I rewrapped it to 72 columns, so I
>>> think the
>>> patch I sent is correctly wrapped.
>>
>>   By my count, the line 'reasonable level of optimization while
>> maintaining fast compilation' is 68 characters long. with the tab + 2
>> spaces that becomes 78.
>>
>
> I was working under the assumption that tabs count as 1 character. If tabs count
> as 8 characters instead, then the rest of the file is already miswrapped; there
> are many lines containing a tab, 2 spaces, and 70 characters after. Under
> BR_OPTIMIZE_2, the line starting with "the performance of the generated code" is
> an example of that. In addition, most text editors seem to count a tab as 1
> character when displaying width (Vim certainly does).
>
> If the intention is to count a tab as 8 characters, I'd be happy to do so, but
> then we should also rewrap the rest of the file for consistency.

  You have a point there. I'll Ack your v3, care to send a follow-up that 
rewraps the entire file?

  Regards,
  Arnout


[snip]


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF