Patchwork Fix PR46037

login
register
mail settings
Submitter Jack Howarth
Date Jan. 11, 2011, 10:52 p.m.
Message ID <20110111225251.GA16941@bromo.med.uc.edu>
Download mbox | patch
Permalink /patch/78463/
State New
Headers show

Comments

Jack Howarth - Jan. 11, 2011, 10:52 p.m.
Current gcc trunk is unable to lto-bootstrap on darwin due to a
bug in darwin_override_options() where debug_hooks->var_location is
tested prematurely for setting flag_var_tracking_uninit. This can be
solved by switching the test for setting flag_var_tracking_uninit to
check write_symbols instead. However the handling of flag_gtoggle
from process_options() in toplev.c needs to be moved into darwin_override_options()
as well to insure that debug_info_level and write_symbols are properly
set before tested. Bootstrap and regression tested with a normal and
lto-bootstrap on x86_64-apple-darwin10. Okay for gcc trunk?
                Jack
ps I also removed the unnecessary parentheses from the test
on generating_for_darwin_version for consistent formatting. These
changes also appear to eliminate the regression...

XPASS: gcc.dg/tree-ssa/20040204-1.c scan-tree-dump-times optimized "link_error" 0

at -m32/-m64 on x86_64-apple-darwin10.

2011-01-11  Jan Hubicka  <jh@suse.cz>
	    Jack Howarth <howarth@bromo.med.uc.edu>

	PR target/46037
	* config/darwin.c (darwin_override_options): Handle flag_gtoggle.
	Test write_symbols instead of debug_hooks->var_location when setting
	flag_var_tracking_uninit.
Mike Stump - Jan. 12, 2011, 12:45 a.m.
On Jan 11, 2011, at 2:52 PM, Jack Howarth wrote:
>   Current gcc trunk is unable to lto-bootstrap on darwin due to a
> bug in darwin_override_options() where debug_hooks->var_location is
> tested prematurely for setting flag_var_tracking_uninit. This can be
> solved by switching the test for setting flag_var_tracking_uninit to
> check write_symbols instead. However the handling of flag_gtoggle
> from process_options() in toplev.c needs to be moved into darwin_override_options()
> as well to insure that debug_info_level and write_symbols are properly
> set before tested. Bootstrap and regression tested with a normal and
> lto-bootstrap on x86_64-apple-darwin10. Okay for gcc trunk?

Ok.


Meta comment: I wish someone had a design for the machine independent parts of the compiler for this fix instead...  Kinda gross in the port files.  :-(  Though, maybe that's just because the darwin port is gross...  Would be nice to have a `cleaner' (a person with global privs that likes to clean up stupid things like this)...  Maybe I could encourage the steering committee to create such a role, I think it would be beneficial, though, possibly, no one would step forward for the thankless job.
Jan Hubicka - Jan. 12, 2011, 2:23 a.m.
> Index: gcc/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c	(revision 168667)
> +++ gcc/config/darwin.c	(working copy)
> @@ -2615,10 +2615,23 @@ darwin_override_options (void)
>        darwin_emit_branch_islands = true;
>      }
>  
> +  if (flag_gtoggle)
> +    {
> +      if (debug_info_level == DINFO_LEVEL_NONE)
> +        {
> +          debug_info_level = DINFO_LEVEL_NORMAL;
> +
> +          if (write_symbols == NO_DEBUG)
> +            write_symbols = PREFERRED_DEBUGGING_TYPE;
> +        }
> +      else
> +        debug_info_level = DINFO_LEVEL_NONE;
> +    }

Won't this result in reverting debug_info_level with -gtoggle each time options are overriden?

Honza
> +
>    if (flag_var_tracking
> -      && (generating_for_darwin_version >= 9)
> +      && generating_for_darwin_version >= 9
>        && debug_info_level >= DINFO_LEVEL_NORMAL
> -      && debug_hooks->var_location != do_nothing_debug_hooks.var_location)
> +      && write_symbols == DWARF2_DEBUG)
>      flag_var_tracking_uninit = 1;
>  
>    if (MACHO_DYNAMIC_NO_PIC_P)
Jack Howarth - Jan. 12, 2011, 2:53 a.m.
On Wed, Jan 12, 2011 at 03:23:09AM +0100, Jan Hubicka wrote:
> > Index: gcc/config/darwin.c
> > ===================================================================
> > --- gcc/config/darwin.c	(revision 168667)
> > +++ gcc/config/darwin.c	(working copy)
> > @@ -2615,10 +2615,23 @@ darwin_override_options (void)
> >        darwin_emit_branch_islands = true;
> >      }
> >  
> > +  if (flag_gtoggle)
> > +    {
> > +      if (debug_info_level == DINFO_LEVEL_NONE)
> > +        {
> > +          debug_info_level = DINFO_LEVEL_NORMAL;
> > +
> > +          if (write_symbols == NO_DEBUG)
> > +            write_symbols = PREFERRED_DEBUGGING_TYPE;
> > +        }
> > +      else
> > +        debug_info_level = DINFO_LEVEL_NONE;
> > +    }
> 
> Won't this result in reverting debug_info_level with -gtoggle each time options are overriden?

Honza,
  You're right. If I try...

[MacPro:~] howarth% g++-4 -g0 -gtoggle -O2 hello.cc
hello.cc:1:0: warning: variable tracking requested, but useless unless producing debug info [enabled by default]

So we are back to trying to figure out how to use change

-      && debug_hooks->var_location != do_nothing_debug_hooks.var_location)
+      && write_symbols == DWARF2_DEBUG)

for the lto-bootstrap without it breaking the normal bootstrap. Sigh.
                 Jack

> 
> Honza
> > +
> >    if (flag_var_tracking
> > -      && (generating_for_darwin_version >= 9)
> > +      && generating_for_darwin_version >= 9
> >        && debug_info_level >= DINFO_LEVEL_NORMAL
> > -      && debug_hooks->var_location != do_nothing_debug_hooks.var_location)
> > +      && write_symbols == DWARF2_DEBUG)
> >      flag_var_tracking_uninit = 1;
> >  
> >    if (MACHO_DYNAMIC_NO_PIC_P)
Gerald Pfeifer - Jan. 17, 2011, 7:45 p.m.
On Tue, 11 Jan 2011, Mike Stump wrote:
> Would be nice to have a `cleaner' (a person with global privs that
> likes to clean up stupid things like this)...

Well, aren't we seeing RTH do just that right now with his current
patch series?  And Joseph around specs?  Just two examples...

> Maybe I could encourage the steering committee to create such a
> role, I think it would be beneficial, though, possibly, no one
> would step forward for the thankless job.

I think the only way this can work is for something to step up
and go for it, and have the formal role follow.  (And, yes, I'll
wholeheartedly support Joseph become spec maintainer.)

Gerald
Mike Stump - Jan. 18, 2011, 7:58 p.m.
On Jan 17, 2011, at 11:45 AM, Gerald Pfeifer wrote:
> I think the only way this can work is for something to step up
> and go for it, and have the formal role follow.  (And, yes, I'll
> wholeheartedly support Joseph become spec maintainer.)

Heck, I'd nominate him for _any_ cleanup job he feels comfortable with.  :-)

Patch

Index: gcc/config/darwin.c
===================================================================
--- gcc/config/darwin.c	(revision 168667)
+++ gcc/config/darwin.c	(working copy)
@@ -2615,10 +2615,23 @@  darwin_override_options (void)
       darwin_emit_branch_islands = true;
     }
 
+  if (flag_gtoggle)
+    {
+      if (debug_info_level == DINFO_LEVEL_NONE)
+        {
+          debug_info_level = DINFO_LEVEL_NORMAL;
+
+          if (write_symbols == NO_DEBUG)
+            write_symbols = PREFERRED_DEBUGGING_TYPE;
+        }
+      else
+        debug_info_level = DINFO_LEVEL_NONE;
+    }
+
   if (flag_var_tracking
-      && (generating_for_darwin_version >= 9)
+      && generating_for_darwin_version >= 9
       && debug_info_level >= DINFO_LEVEL_NORMAL
-      && debug_hooks->var_location != do_nothing_debug_hooks.var_location)
+      && write_symbols == DWARF2_DEBUG)
     flag_var_tracking_uninit = 1;
 
   if (MACHO_DYNAMIC_NO_PIC_P)