Re: [Xen-devel] [PATCH 2/2] tools: detect appropriate debug optimization level

[George Dunlap <dunlapg@xxxxxxxxx>]

On Tue, Mar 29, 2016 at 6:21 PM, Doug Goldstein <cardoe@xxxxxxxxxx> wrote:
> On 3/29/16 6:44 AM, George Dunlap wrote:
>> On Mon, Mar 28, 2016 at 4:01 PM, Doug Goldstein <cardoe@xxxxxxxxxx> wrote:
>>> On 3/16/16 2:14 PM, Doug Goldstein wrote:
>>>> On 3/8/16 10:50 AM, Wei Liu wrote:
>>>>> On Tue, Mar 08, 2016 at 10:34:42AM -0600, Doug Goldstein wrote:
>>>>>> On 3/8/16 9:38 AM, Wei Liu wrote:
>>>>>>> On Mon, Mar 07, 2016 at 08:23:40PM -0600, Doug Goldstein wrote:
>>>>>>>> The build should not use -O0 as that results in miscompilations. There
>>>>>>>
>>>>>>> This needs some (concrete) references. Is that a known issue in gcc? If
>>>>>>> so can you reference the bug number?
>>>>>>
>>>>>> So its not really a bug in GCC but just the complete lack of
>>>>>> optimizations in play. inlines aren't inlined. dead code elimination
>>>>>> isn't run so things are much bigger. structures aren't padded the same 
>>>>>> way.
>>>>>>
>>>>>
>>>>> Urgh...
>>>>>
>>>>>> This came about from reading reports on the -devel and -user's ML that
>>>>>> were solved by building Xen with debug=n. I was also striving to reduce
>>>>>> the duplication of CFLAGS that are passed on the command line of builds.
>>>>>>
>>>>>
>>>>> I agree this is a good idea.
>>>>>
>>>>>>>
>>>>>>>> have been a few instances on the ML where users were told to switch
>>>>>>>> from -O0 to -O1 or -O2 or to set debug=n and their issue went away. The
>>>>>>>> preferred route should be to use -Og if its available, otherwise use
>>>>>>>> -O1 which is the default. This change undoes the change from -O1 to -O0
>>>>>>>
>>>>>>> gcc manual says -O0 is the default.
>>>>>>
>>>>>> I wasn't clear about where the 'the default' came from. That's the
>>>>>> default in the Xen tree (see: config/StdGNU.mk for example but every
>>>>>> platform has -O1 set).
>>>>>>
>>>>>
>>>>> OK. I thought you're talking about something in the manual.
>>>>>
>>>>>>>
>>>>>>> Not that I disagree with this patch in general, but the commit message
>>>>>>> seems a bit misleading.
>>>>>>
>>>>>> I can rewrite it. I'd also be willing to change the patch to prefer -Og
>>>>>> if its available and use -O0 if its not.
>>>>>>
>>>>>
>>>>> No need to do it now because ...
>>>>>
>>>>>>>
>>>>>>>> in 1166ecf781b1016eaa61f8d5ba4fb1fde9d599b6.
>>>>>>>>
>>>>>>>
>>>>>>> And I have no idea why -O1 confuses the debugger so I've CC'ed Euan for
>>>>>>> more input.
>>>>>>
>>>>>> -O1 can optimize things out when you look at them with gdb but -Og is
>>>>>> suppose to do the right thing.
>>>>>>
>>>>>
>>>>> .. I don't know much about gcc so I would like to wait for Ian to give
>>>>> some input.
>>>>>
>>>>> Wei.
>>>>>
>>>>>>>
>>>>>>>> Signed-off-by: Doug Goldstein <cardoe@xxxxxxxxxx>
>>>>>>>> ---
>>>>>>>> CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>>>>>>>> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>>>>>>>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>>>>> ---
>>>>>>>>  tools/Rules.mk | 3 ++-
>>>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>>>>>>> index 9ef0b47..ae6b01f 100644
>>>>>>>> --- a/tools/Rules.mk
>>>>>>>> +++ b/tools/Rules.mk
>>>>>>>> @@ -137,7 +137,8 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) 
>>>>>>>> -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>>>>>>>
>>>>>>>>  ifeq ($(debug),y)
>>>>>>>>  # Disable optimizations and enable debugging information for macros
>>>>>>>> -CFLAGS += -O0 -g3
>>>>>>>> +$(call cc-option-add,CFLAGS,CC,-Og)
>>>>>>>> +CFLAGS += -g3
>>>>>>>>  # But allow an override to -O0 in case Python enforces 
>>>>>>>> -D_FORTIFY_SOURCE=<n>.
>>>>>>>>  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
>>>>>>>>  endif
>>>>>>>> --
>>>>>>>> 2.4.10
>>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Doug Goldstein
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>> ping?
>>>>
>>>>
>>>
>>> ping the ping?
>>
>> So just reading through the history -- I'm a bit confused why, if -Og
>> is supposed to "do the right thing", why you didn't add that in this
>> patch?
>
> I did.
>
> +$(call cc-option-add,CFLAGS,CC,-Og)
>
> That tests to see if the version of GCC you're using supports that flag
> and if it does adds it. Otherwise it does nothing. Per the README we
> support down to GCC 4.1.2 and -Og was added in 4.7 I believe. For
> versions of GCC older than 4.7 this uses the debug=y default of -O1
> instead of using -O0 which is known to break in some cases.

Gah -- sorry, my eyes skipped over that when skimming this thread.
Sorry about that. :-)

So it looks like we're still waiting for someone who knows more about
gcc to comment.

Andy, you seems to know this sort of thing -- would you be willing
take a look?  Otherwise we may have to ping IanJ out-of-band.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel