All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Kelly <martin@surround.io>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] Config.in: add -Og option
Date: Mon, 16 May 2016 16:36:53 -0700	[thread overview]
Message-ID: <573A5995.6020201@surround.io> (raw)
In-Reply-To: <20160514142558.70fb92f4@free-electrons.com>

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.

  reply	other threads:[~2016-05-16 23:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-13 23:57 [Buildroot] [PATCH] Config.in: add -Og option Martin Kelly
2016-05-14 12:25 ` Thomas Petazzoni
2016-05-16 23:36   ` Martin Kelly [this message]
2016-05-17 23:54     ` Martin Kelly
2016-05-14 22:50 ` Arnout Vandecappelle
2016-05-16 23:37   ` Martin Kelly

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=573A5995.6020201@surround.io \
    --to=martin@surround.io \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.