diff mbox

PR61868

Message ID B71DF1153024A14EABB94E39368E44A6042D63F4@SJEXCHMB13.corp.ad.broadcom.com
State New
Headers show

Commit Message

Bingfeng Mei July 30, 2014, 3:20 p.m. UTC 
Yes, that fix is better. Here are updated patches. LTO-bootstrapped and tested. OK?

Bingfeng


> -----Original Message-----

> From: Richard Biener [mailto:richard.guenther@gmail.com]

> Sent: 30 July 2014 14:08

> To: Bingfeng Mei

> Cc: Andi Kleen; gcc-patches@gcc.gnu.org

> Subject: Re: [PATCH] PR61868

> 

> On Wed, Jul 30, 2014 at 2:48 PM, Bingfeng Mei <bmei@broadcom.com> wrote:

> > On the second thought, maybe the change should be made in

> set_random_seed. Otherwise, init_random_seed is invoked every time

> get_random_seed get called (not sure how often). What do you think?

> >

> > const char *

> > set_random_seed (const char *val)

> > {

> >   const char *old = flag_random_seed;

> >   flag_random_seed = val;

> > + init_random_seed ();

> >   return old;

> > }

> 

> I think it makes more sense to move the if (flag_random_seed)

> code from init_random_seed inline here as init_random_seed is

> never called with flag_random_set == true.

> 

> Richard.

> 

> > Bingfeng

> >

> >> -----Original Message-----

> >> From: Richard Biener [mailto:richard.guenther@gmail.com]

> >> Sent: 30 July 2014 10:43

> >> To: Bingfeng Mei

> >> Cc: Andi Kleen; gcc-patches@gcc.gnu.org

> >> Subject: Re: [PATCH] PR61868

> >>

> >> On Wed, Jul 30, 2014 at 10:52 AM, Bingfeng Mei <bmei@broadcom.com>

> wrote:

> >> > Yes, opts-global.c fills flag_random_seed with string passed in

> >> command line. But init_random_seed in toplev.c is the one that

> processes

> >> the flag and set random_seed variable, which is returned by

> >> get_random_seed function.

> >>

> >> Err, I meant opts-global.c:handle_common_deferred_options

> >>

> >> > By default, my configuration enables LTO. How to test LTO bootstrap?

> >>

> >> configure with --with-build-config=bootstrap-lto (and reduce the

> >> set of languages to c,c++ to get a sane bootstrap time).

> >>

> >> Richard.

> >>

> >> > Bingfeng

> >> >

> >> >> -----Original Message-----

> >> >> From: Richard Biener [mailto:richard.guenther@gmail.com]

> >> >> Sent: 30 July 2014 08:47

> >> >> To: Bingfeng Mei; Andi Kleen

> >> >> Cc: gcc-patches@gcc.gnu.org

> >> >> Subject: Re: [PATCH] PR61868

> >> >>

> >> >> On Tue, Jul 29, 2014 at 3:01 PM, Bingfeng Mei <bmei@broadcom.com>

> >> wrote:

> >> >> > Hi,

> >> >> > Please find the patches for pr61868. The problem is that even

> with

> >> -

> >> >> frandom-seed gcc always uses 0 instead of specified number/string

> for

> >> >> places such as lto section names. init_random_seed is never called

> >> with

> >> >> -frandom-seed. Bootstrapped and tested on x86-64. OK for trunk?

> >> >>

> >> >> Hmm, but -frandom-seed= is honored by opts.c calling

> set_random_seed?

> >> >>

> >> >> Andi, wasn't this used to support partial linking of LTO objects?

> >> Thus,

> >> >> this shortcut was on purpose?  Otherwise passing -frandom-seed to

> >> >> LTO would break that?

> >> >>

> >> >> Bingfeng, did you test if LTO bootstrap still works after this

> patch?

> >> >>

> >> >> Thanks,

> >> >> Richard.

> >> >>

> >> >> > Thanks,

> >> >> > Bingfeng Mei

> >> >> >

> >> >> >

> >> >> >

> ===================================================================

> >> >> > --- ChangeLog   (revision 213152)

> >> >> > +++ ChangeLog   (working copy)

> >> >> > @@ -1,3 +1,9 @@

> >> >> > +2014-07-29  Bingfeng Mei <bmei@broadcom.com>

> >> >> > +

> >> >> > +       PR lto/61868

> >> >> > +       * toplev.c (init_random_seed): Generate random seed when

> >> >> > +       flag_random_seed is specified.

> >> >> > +

> >> >> >  2014-07-28  Jan Hubicka  <hubicka@ucw.cz>

> >> >> >

> >> >> >         * cgraph.c (cgraph_node::create_indirect_edge): Copy

> >> >> speculative data.

> >> >> > Index: toplev.c

> >> >> >

> ===================================================================

> >> >> > --- toplev.c    (revision 213152)

> >> >> > +++ toplev.c    (working copy)

> >> >> > @@ -301,7 +301,7 @@ init_random_seed (void)

> >> >> >  HOST_WIDE_INT

> >> >> >  get_random_seed (bool noinit)

> >> >> >  {

> >> >> > -  if (!flag_random_seed && !noinit)

> >> >> > +  if (!noinit)

> >> >> >      init_random_seed ();

> >> >> >    return random_seed;

> >> >> >  }

> >> >> > Index: testsuite/ChangeLog

> >> >> >

> ===================================================================

> >> >> > --- testsuite/ChangeLog (revision 213152)

> >> >> > +++ testsuite/ChangeLog (working copy)

> >> >> > @@ -1,3 +1,8 @@

> >> >> > +2014-07-29  Bingfeng Mei  <bmei@broadcom.com>

> >> >> > +

> >> >> > +       PR lto/61868

> >> >> > +       * gcc.dg/pr61868.c: New test.

> >> >> > +

> >> >> >  2014-07-28  Richard Biener  <rguenther@suse.de>

> >> >> >

> >> >> >         PR rtl-optimization/61801

> >> >> > Index: testsuite/gcc.dg/pr61868.c

> >> >> >

> ===================================================================

> >> >> > --- testsuite/gcc.dg/pr61868.c  (revision 0)

> >> >> > +++ testsuite/gcc.dg/pr61868.c  (revision 0)

> >> >> > @@ -0,0 +1,9 @@

> >> >> > +/* { dg-do compile } */

> >> >> > +/* { dg-options "-flto -frandom-seed=0x12345" }  */

> >> >> > +extern int foo (int);

> >> >> > +int main ()

> >> >> > +{

> >> >> > +  foo (100);

> >> >> > +  return 0;

> >> >> > +}

> >> >> > +/* { dg-final { scan-assembler "\.gnu\.lto.*.12345" } } */

> >> >> >

Comments

Jeff Law July 31, 2014, 3:43 a.m. UTC | #1 
On 07/30/14 09:20, Bingfeng Mei wrote:
> Yes, that fix is better. Here are updated patches. LTO-bootstrapped and tested. OK?
>
> Bingfeng
>
> Index: ChangeLog
> ===================================================================
> --- ChangeLog   (revision 213152)
> +++ ChangeLog   (working copy)
> @@ -1,3 +1,10 @@
> +2014-07-29  Bingfeng Mei <bmei@broadcom.com>
> +
> +       PR lto/61868
> +       * toplev.c (init_random_seed): Move piece of code never called to
> +       set_random_seed.
> +       (set_random_seed): see above.
> +
>   2014-07-28  Jan Hubicka  <hubicka@ucw.cz>
OK for the trunk.

Thanks,
Jeff
Bingfeng Mei July 31, 2014, 8:52 a.m. UTC | #2 
Thanks, Jeff. Checked in. 

Bingfeng

> -----Original Message-----

> From: Jeff Law [mailto:law@redhat.com]

> Sent: 31 July 2014 04:43

> To: Bingfeng Mei; Richard Biener

> Cc: Andi Kleen; gcc-patches@gcc.gnu.org

> Subject: Re: [PATCH] PR61868

> 

> On 07/30/14 09:20, Bingfeng Mei wrote:

> > Yes, that fix is better. Here are updated patches. LTO-bootstrapped

> and tested. OK?

> >

> > Bingfeng

> >

> > Index: ChangeLog

> > ===================================================================

> > --- ChangeLog   (revision 213152)

> > +++ ChangeLog   (working copy)

> > @@ -1,3 +1,10 @@

> > +2014-07-29  Bingfeng Mei <bmei@broadcom.com>

> > +

> > +       PR lto/61868

> > +       * toplev.c (init_random_seed): Move piece of code never called

> to

> > +       set_random_seed.

> > +       (set_random_seed): see above.

> > +

> >   2014-07-28  Jan Hubicka  <hubicka@ucw.cz>

> OK for the trunk.

> 

> Thanks,

> Jeff
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 213152)
+++ ChangeLog   (working copy)
@@ -1,3 +1,10 @@ 
+2014-07-29  Bingfeng Mei <bmei@broadcom.com>
+
+       PR lto/61868
+       * toplev.c (init_random_seed): Move piece of code never called to
+       set_random_seed.
+       (set_random_seed): see above.
+
 2014-07-28  Jan Hubicka  <hubicka@ucw.cz>

        * cgraph.c (cgraph_node::create_indirect_edge): Copy speculative data.
Index: toplev.c
===================================================================
--- toplev.c    (revision 213152)
+++ toplev.c    (working copy)
@@ -282,16 +282,7 @@  init_local_tick (void)
 static void
 init_random_seed (void)
 {
-  if (flag_random_seed)
-    {
-      char *endp;
-
-      /* When the driver passed in a hex number don't crc it again */
-      random_seed = strtoul (flag_random_seed, &endp, 0);
-      if (!(endp > flag_random_seed && *endp == 0))
-        random_seed = crc32_string (0, flag_random_seed);
-    }
-  else if (!random_seed)
+  if (!random_seed)
     random_seed = local_tick ^ getpid ();  /* Old racey fallback method */
 }

@@ -314,6 +305,15 @@  set_random_seed (const char *val)
 {
   const char *old = flag_random_seed;
   flag_random_seed = val;
+  if (flag_random_seed)
+    {
+      char *endp;
+
+      /* When the driver passed in a hex number don't crc it again */
+      random_seed = strtoul (flag_random_seed, &endp, 0);
+      if (!(endp > flag_random_seed && *endp == 0))
+        random_seed = crc32_string (0, flag_random_seed);
+    }
   return old;
 }

Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 213152)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@ 
+2014-07-29  Bingfeng Mei  <bmei@broadcom.com>
+
+       PR lto/61868
+       * gcc.dg/pr61868.c: New test.
+
 2014-07-28  Richard Biener  <rguenther@suse.de>

        PR rtl-optimization/61801
Index: testsuite/gcc.dg/pr61868.c
===================================================================
--- testsuite/gcc.dg/pr61868.c  (revision 0)
+++ testsuite/gcc.dg/pr61868.c  (revision 0)
@@ -0,0 +1,9 @@ 
+/* { dg-do compile } */
+/* { dg-options "-flto -frandom-seed=0x12345" }  */
+extern int foo (int);
+int main ()
+{
+  foo (100);
+  return 0;
+}
+/* { dg-final { scan-assembler "\.gnu\.lto.*.12345" } } */