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

[George Dunlap <dunlapg@umich.edu>]

On Mon, Mar 28, 2016 at 4:01 PM, Doug Goldstein <cardoe@cardoe.com> 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@cardoe.com>
>>>>>> ---
>>>>>> CC: Ian Jackson <ian.jackson@eu.citrix.com>
>>>>>> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>>>> CC: Wei Liu <wei.liu2@citrix.com>
>>>>>> ---
>>>>>>  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?

In any case, having debug=y *functioning* should take priority over
having gdb working, so I'm inclined to say that we should take a patch
like this.

 -George

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