xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware
@ 2016-03-08  2:23 Doug Goldstein
  2016-03-08  2:23 ` [PATCH 2/2] tools: detect appropriate debug optimization level Doug Goldstein
  2016-03-08 15:38 ` [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Wei Liu
  0 siblings, 2 replies; 19+ messages in thread
From: Doug Goldstein @ 2016-03-08  2:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Doug Goldstein, Wei Liu, Stefano Stabellini

Some versions of GCC complain that the 'firmware' variable can be used
uninitialized. It looks like the switch inside of the else case is just
confusing GCC.

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/libxl/libxl_dom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 664adad..b825b98 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -867,7 +867,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
                                   struct xc_dom_image *dom)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    const char *firmware;
+    const char *firmware = NULL;
     int e, rc;
     int datalen = 0;
     void *data;
-- 
2.4.10


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08  2:23 [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Doug Goldstein
@ 2016-03-08  2:23 ` Doug Goldstein
  2016-03-08 15:38   ` Wei Liu
  2016-04-06 14:07   ` Ian Jackson
  2016-03-08 15:38 ` [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Wei Liu
  1 sibling, 2 replies; 19+ messages in thread
From: Doug Goldstein @ 2016-03-08  2:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Doug Goldstein, Wei Liu, Stefano Stabellini

The build should not use -O0 as that results in miscompilations. There
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
in 1166ecf781b1016eaa61f8d5ba4fb1fde9d599b6.

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


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08  2:23 ` [PATCH 2/2] tools: detect appropriate debug optimization level Doug Goldstein
@ 2016-03-08 15:38   ` Wei Liu
  2016-03-08 16:34     ` Doug Goldstein
  2016-04-06 14:07   ` Ian Jackson
  1 sibling, 1 reply; 19+ messages in thread
From: Wei Liu @ 2016-03-08 15:38 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: euan.harris, Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel

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?

> 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.

Not that I disagree with this patch in general, but the commit message
seems a bit misleading.

> in 1166ecf781b1016eaa61f8d5ba4fb1fde9d599b6.
> 

And I have no idea why -O1 confuses the debugger so I've CC'ed Euan for
more input.

> 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
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware
  2016-03-08  2:23 [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Doug Goldstein
  2016-03-08  2:23 ` [PATCH 2/2] tools: detect appropriate debug optimization level Doug Goldstein
@ 2016-03-08 15:38 ` Wei Liu
  2016-03-10 15:13   ` Doug Goldstein
  1 sibling, 1 reply; 19+ messages in thread
From: Wei Liu @ 2016-03-08 15:38 UTC (permalink / raw)
  To: Doug Goldstein; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel

On Mon, Mar 07, 2016 at 08:23:39PM -0600, Doug Goldstein wrote:
> Some versions of GCC complain that the 'firmware' variable can be used
> uninitialized. It looks like the switch inside of the else case is just
> confusing GCC.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

Acked-by: Wei Liu <wei.liu2@citrix.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/libxl/libxl_dom.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 664adad..b825b98 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -867,7 +867,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
>                                    struct xc_dom_image *dom)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
> -    const char *firmware;
> +    const char *firmware = NULL;
>      int e, rc;
>      int datalen = 0;
>      void *data;
> -- 
> 2.4.10
> 

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08 15:38   ` Wei Liu
@ 2016-03-08 16:34     ` Doug Goldstein
  2016-03-08 16:50       ` Wei Liu
  2016-03-30 16:00       ` Ian Jackson
  0 siblings, 2 replies; 19+ messages in thread
From: Doug Goldstein @ 2016-03-08 16:34 UTC (permalink / raw)
  To: Wei Liu; +Cc: euan.harris, Stefano Stabellini, Ian Jackson, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2626 bytes --]

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.

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.

> 
>> 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).

> 
> 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.

> 
>> 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.

> 
>> 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


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08 16:34     ` Doug Goldstein
@ 2016-03-08 16:50       ` Wei Liu
  2016-03-16 19:14         ` Doug Goldstein
  2016-03-30 16:00       ` Ian Jackson
  1 sibling, 1 reply; 19+ messages in thread
From: Wei Liu @ 2016-03-08 16:50 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: euan.harris, Ian Jackson, Stefano Stabellini, Wei Liu, xen-devel

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
> 




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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware
  2016-03-08 15:38 ` [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Wei Liu
@ 2016-03-10 15:13   ` Doug Goldstein
  0 siblings, 0 replies; 19+ messages in thread
From: Doug Goldstein @ 2016-03-10 15:13 UTC (permalink / raw)
  To: Wei Liu; +Cc: Stefano Stabellini, Ian Jackson, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 520 bytes --]

On 3/8/16 9:38 AM, Wei Liu wrote:
> On Mon, Mar 07, 2016 at 08:23:39PM -0600, Doug Goldstein wrote:
>> Some versions of GCC complain that the 'firmware' variable can be used
>> uninitialized. It looks like the switch inside of the else case is just
>> confusing GCC.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 

I sent these two together but they really aren't dependent on each other
so this can go in without the other.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08 16:50       ` Wei Liu
@ 2016-03-16 19:14         ` Doug Goldstein
  2016-03-28 15:01           ` Doug Goldstein
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Goldstein @ 2016-03-16 19:14 UTC (permalink / raw)
  To: Wei Liu; +Cc: euan.harris, Stefano Stabellini, Ian Jackson, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 3221 bytes --]

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?


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-16 19:14         ` Doug Goldstein
@ 2016-03-28 15:01           ` Doug Goldstein
  2016-03-29 11:44             ` George Dunlap
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Goldstein @ 2016-03-28 15:01 UTC (permalink / raw)
  To: Wei Liu; +Cc: euan.harris, Stefano Stabellini, Ian Jackson, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 3378 bytes --]

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?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-28 15:01           ` Doug Goldstein
@ 2016-03-29 11:44             ` George Dunlap
  2016-03-29 17:21               ` Doug Goldstein
  0 siblings, 1 reply; 19+ messages in thread
From: George Dunlap @ 2016-03-29 11:44 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: Euan Harris, xen-devel, Wei Liu, Ian Jackson, Stefano Stabellini

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-29 11:44             ` George Dunlap
@ 2016-03-29 17:21               ` Doug Goldstein
  2016-03-30  9:52                 ` George Dunlap
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Goldstein @ 2016-03-29 17:21 UTC (permalink / raw)
  To: George Dunlap
  Cc: Euan Harris, xen-devel, Wei Liu, Ian Jackson, Stefano Stabellini


[-- Attachment #1.1.1: Type: text/plain, Size: 4487 bytes --]

On 3/29/16 6:44 AM, George Dunlap wrote:
> 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?

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.

> 
> 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.

Exactly why I'd like to see this land.

> 
>  -George
> 


-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-29 17:21               ` Doug Goldstein
@ 2016-03-30  9:52                 ` George Dunlap
  0 siblings, 0 replies; 19+ messages in thread
From: George Dunlap @ 2016-03-30  9:52 UTC (permalink / raw)
  To: Doug Goldstein
  Cc: Wei Liu, Stefano Stabellini, Andrew Cooper, Ian Jackson,
	xen-devel, Euan Harris

On Tue, Mar 29, 2016 at 6:21 PM, Doug Goldstein <cardoe@cardoe.com> wrote:
> On 3/29/16 6:44 AM, George Dunlap wrote:
>> 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?
>
> 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@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08 16:34     ` Doug Goldstein
  2016-03-08 16:50       ` Wei Liu
@ 2016-03-30 16:00       ` Ian Jackson
  2016-03-30 16:11         ` Ian Jackson
                           ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Ian Jackson @ 2016-03-30 16:00 UTC (permalink / raw)
  To: Doug Goldstein; +Cc: euan.harris, Stefano Stabellini, Wei Liu, xen-devel

Doug Goldstein writes ("Re: [PATCH 2/2] tools: detect appropriate debug optimization level"):
> 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.

My initial reaction is that I any actual problems are bugs either in
the compiler or in Xen, which should be fixed.

There should be nothing wrong with lack of inlining or dead code
elimination.  If you can give an example of structure padding going
wrong, please do.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-30 16:00       ` Ian Jackson
@ 2016-03-30 16:11         ` Ian Jackson
  2016-03-30 16:20         ` Doug Goldstein
  2016-03-31  8:24         ` George Dunlap
  2 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2016-03-30 16:11 UTC (permalink / raw)
  To: Doug Goldstein, Wei Liu, xen-devel, Stefano Stabellini, euan.harris

Ian Jackson writes ("Re: [PATCH 2/2] tools: detect appropriate debug optimization level"):
> My initial reaction is that I any actual problems are bugs either in
> the compiler or in Xen, which should be fixed.
> 
> There should be nothing wrong with lack of inlining or dead code
> elimination.  If you can give an example of structure padding going
> wrong, please do.

Having said that, the reason for specifying -O0 is the use case that
gcc now provides -Og for.  So I see no harm and some benefit in using
-Og if it is supported.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-30 16:00       ` Ian Jackson
  2016-03-30 16:11         ` Ian Jackson
@ 2016-03-30 16:20         ` Doug Goldstein
  2016-03-31  8:24         ` George Dunlap
  2 siblings, 0 replies; 19+ messages in thread
From: Doug Goldstein @ 2016-03-30 16:20 UTC (permalink / raw)
  To: Ian Jackson; +Cc: euan.harris, Stefano Stabellini, Wei Liu, xen-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1404 bytes --]

On 3/30/16 11:00 AM, Ian Jackson wrote:
> Doug Goldstein writes ("Re: [PATCH 2/2] tools: detect appropriate debug optimization level"):
>> 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.
> 
> My initial reaction is that I any actual problems are bugs either in
> the compiler or in Xen, which should be fixed.
> 
> There should be nothing wrong with lack of inlining or dead code
> elimination.  If you can give an example of structure padding going
> wrong, please do.
> 
> Ian.
> 

Ok fine, I'm just confused why we're insisting on using -O0 over -Og?

From the gcc manual:

-Og
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.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-30 16:00       ` Ian Jackson
  2016-03-30 16:11         ` Ian Jackson
  2016-03-30 16:20         ` Doug Goldstein
@ 2016-03-31  8:24         ` George Dunlap
  2 siblings, 0 replies; 19+ messages in thread
From: George Dunlap @ 2016-03-31  8:24 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Euan Harris, xen-devel, Doug Goldstein, Wei Liu, Stefano Stabellini

On Wed, Mar 30, 2016 at 5:00 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Doug Goldstein writes ("Re: [PATCH 2/2] tools: detect appropriate debug optimization level"):
>> 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.
>
> My initial reaction is that I any actual problems are bugs either in
> the compiler or in Xen, which should be fixed.
>
> There should be nothing wrong with lack of inlining or dead code
> elimination.  If you can give an example of structure padding going
> wrong, please do.

I know in the Linux kernel the level of optimization must be -O2,
because there are certain things that rely on dead code elimination to
function properly.  I'm not sure if the Xen hypervisor has similar
requirements, but I'd be rather surprised if it didn't.

It would be nice to know what functionality in the tools relied on
inlining and/or dead code elimination (and under what circumstances),
but we probably have better things to do than make -O0 to work. :-)

 -George

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-03-08  2:23 ` [PATCH 2/2] tools: detect appropriate debug optimization level Doug Goldstein
  2016-03-08 15:38   ` Wei Liu
@ 2016-04-06 14:07   ` Ian Jackson
  2016-04-06 14:34     ` George Dunlap
  1 sibling, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2016-04-06 14:07 UTC (permalink / raw)
  To: Doug Goldstein; +Cc: Stefano Stabellini, Wei Liu, xen-devel

Doug Goldstein writes ("[PATCH 2/2] tools: detect appropriate debug optimization level"):
> The build should not use -O0 as that results in miscompilations. There
> 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
> in 1166ecf781b1016eaa61f8d5ba4fb1fde9d599b6.

To summarise: I agree with using -Og if it is available.  But I
disagree with reverting 1166ecf7 in the case where it isn't; if -Og is
not available, we should stick with -O0.

I'm open to persuasion in the latter point but I would like specific
examples of problems (and then I would form an opinion about the
specific problems), not generalities.

Doug, would you like to, for now, propose a patch that uses -Og if it
is available, but otherwise falls back to -O0 ?  NB that if
convenient, this can be done simply by always specifying -O0 and
putting -Og after it if it is supported.

Thanks,
Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-04-06 14:07   ` Ian Jackson
@ 2016-04-06 14:34     ` George Dunlap
  2016-04-06 14:57       ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: George Dunlap @ 2016-04-06 14:34 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Wei Liu, xen-devel, Doug Goldstein, Stefano Stabellini

On Wed, Apr 6, 2016 at 3:07 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Doug Goldstein writes ("[PATCH 2/2] tools: detect appropriate debug optimization level"):
>> The build should not use -O0 as that results in miscompilations. There
>> 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
>> in 1166ecf781b1016eaa61f8d5ba4fb1fde9d599b6.
>
> To summarise: I agree with using -Og if it is available.  But I
> disagree with reverting 1166ecf7 in the case where it isn't; if -Og is
> not available, we should stick with -O0.
>
> I'm open to persuasion in the latter point but I would like specific
> examples of problems (and then I would form an opinion about the
> specific problems), not generalities.
>
> Doug, would you like to, for now, propose a patch that uses -Og if it
> is available, but otherwise falls back to -O0 ?  NB that if
> convenient, this can be done simply by always specifying -O0 and
> putting -Og after it if it is supported.

I agree that it would be good to include specific bugs that this
fixes.  Presuming that they are as described (compilation errors for
reasonable setups), I continue to think that making things compile for
everyone is more important than making them debuggable via gdb for
people running compilers that don't support -Og.

 -George

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] tools: detect appropriate debug optimization level
  2016-04-06 14:34     ` George Dunlap
@ 2016-04-06 14:57       ` Ian Jackson
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2016-04-06 14:57 UTC (permalink / raw)
  To: George Dunlap; +Cc: Wei Liu, xen-devel, Doug Goldstein, Stefano Stabellini

George Dunlap writes ("Re: [Xen-devel] [PATCH 2/2] tools: detect appropriate debug optimization level"):
> On Wed, Apr 6, 2016 at 3:07 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> > Doug, would you like to, for now, propose a patch that uses -Og if it
> > is available, but otherwise falls back to -O0 ?  NB that if
> > convenient, this can be done simply by always specifying -O0 and
> > putting -Og after it if it is supported.
> 
> I agree that it would be good to include specific bugs that this
> fixes.  Presuming that they are as described (compilation errors for
> reasonable setups), I continue to think that making things compile for
> everyone is more important than making them debuggable via gdb for
> people running compilers that don't support -Og.

I think making debug builds debuggable for people with non-broken
compilers is more important than making debug builds build for people
with broken compilers.

(Under some unstated but I think reasonable, and rebuttable,
assumptions about the sizes of the two sets of people.)

Non-debug builds are, after all, not affected by any of this.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-04-06 14:57 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-08  2:23 [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Doug Goldstein
2016-03-08  2:23 ` [PATCH 2/2] tools: detect appropriate debug optimization level Doug Goldstein
2016-03-08 15:38   ` Wei Liu
2016-03-08 16:34     ` Doug Goldstein
2016-03-08 16:50       ` Wei Liu
2016-03-16 19:14         ` Doug Goldstein
2016-03-28 15:01           ` Doug Goldstein
2016-03-29 11:44             ` George Dunlap
2016-03-29 17:21               ` Doug Goldstein
2016-03-30  9:52                 ` George Dunlap
2016-03-30 16:00       ` Ian Jackson
2016-03-30 16:11         ` Ian Jackson
2016-03-30 16:20         ` Doug Goldstein
2016-03-31  8:24         ` George Dunlap
2016-04-06 14:07   ` Ian Jackson
2016-04-06 14:34     ` George Dunlap
2016-04-06 14:57       ` Ian Jackson
2016-03-08 15:38 ` [PATCH 1/2] libxl: ensure var is inited in libxl__domain_firmware Wei Liu
2016-03-10 15:13   ` Doug Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).