[patch] Fix ICE during SCC hashing in LTO mode

[patch] Fix ICE during SCC hashing in LTO mode

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]

Hi,

the attached Ada testcase triggers an internal error in LTO mode:

eric@polaris:~/build/gcc/native> gcc/gnat1 -quiet lto17.adb -flto
+===========================GNAT BUG DETECTED==============================+
| 6.0.0 20150629 (experimental) [trunk revision 225111] (x86_64-suse-linux) 
GCC error:|
| in hash_scc, at lto-streamer-out.c:1445      

This is the assertion verifying that you walk the same SCC using DFS after 
starting from another element of the SCC in hash_scc.  It fails here because 
this second DFS is not performed with the same THIS_REF_P parameter as the 
initial one, so there is no guaranteee that this will yield the same result 
because DFS_write_tree will behave differently.

Fixed by passing the THIS_REF_P and REF_P parameters for FIRST to hash_scc.

Tested on x86_64-suse-linux, OK for the mainline?


2015-06-30  Eric Botcazou  <ebotcazou@adacore.com>

	* lto-streamer-out.c (class DFS): Adjust hash_scc method.
	(DFS::DFS): Pass this_ref_p and ref_p to hash_scc.
	(hash_scc): Add this_ref_p and ref_p parameters and pass them
	to the inner DFS walk.


2015-06-30  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/lto17.ad[sb]: New test.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 3853 bytes --]

Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 225111)
+++ lto-streamer-out.c	(working copy)
@@ -411,9 +411,9 @@ lto_write_tree (struct output_block *ob,
   streamer_write_zero (ob);
 }
 
-/* Emit the physical representation of tree node EXPR to output block
-   OB.  If THIS_REF_P is true, the leaves of EXPR are emitted as references
-   via lto_output_tree_ref.  REF_P is used for streaming siblings of EXPR.  */
+/* Emit the physical representation of tree node EXPR to output block OB,
+   If THIS_REF_P is true, the leaves of EXPR are emitted as references via
+   lto_output_tree_ref.  REF_P is used for streaming siblings of EXPR.  */
 
 static void
 lto_output_tree_1 (struct output_block *ob, tree expr, hashval_t hash,
@@ -490,13 +490,20 @@ private:
 		       tree expr, bool ref_p, bool this_ref_p);
 
   hashval_t
-  hash_scc (struct output_block *ob, unsigned first, unsigned size);
+  hash_scc (struct output_block *ob, unsigned first, unsigned size,
+	    bool ref_p, bool this_ref_p);
 
   hash_map<tree, sccs *> sccstate;
   vec<worklist> worklist_vec;
   struct obstack sccstate_obstack;
 };
 
+/* Emit the physical representation of tree node EXPR to output block OB,
+   using depth-first search on the subgraph.  If THIS_REF_P is true, the
+   leaves of EXPR are emitted as references via lto_output_tree_ref.
+   REF_P is used for streaming siblings of EXPR.  If SINGLE_P is true,
+   this is for a rewalk of a single leaf SCC.  */
+
 DFS::DFS (struct output_block *ob, tree expr, bool ref_p, bool this_ref_p,
 	  bool single_p)
 {
@@ -564,7 +571,7 @@ DFS::DFS (struct output_block *ob, tree
 	  unsigned first, size;
 	  tree x;
 
-	  /* If we are re-walking a single leaf-SCC just pop it,
+	  /* If we are re-walking a single leaf SCC just pop it,
 	     let earlier worklist item access the sccstack.  */
 	  if (single_p)
 	    {
@@ -587,7 +594,7 @@ DFS::DFS (struct output_block *ob, tree
 	  unsigned scc_entry_len = 0;
 	  if (!flag_wpa)
 	    {
-	      scc_hash = hash_scc (ob, first, size);
+	      scc_hash = hash_scc (ob, first, size, ref_p, this_ref_p);
 
 	      /* Put the entries with the least number of collisions first.  */
 	      unsigned entry_start = 0;
@@ -1362,10 +1369,12 @@ DFS::scc_entry_compare (const void *p1_,
   return 0;
 }
 
-/* Return a hash value for the SCC on the SCC stack from FIRST with SIZE.  */
+/* Return a hash value for the SCC on the SCC stack from FIRST with SIZE.
+   THIS_REF_P and REF_P are as passed to lto_output_tree for FIRST.  */
 
 hashval_t
-DFS::hash_scc (struct output_block *ob, unsigned first, unsigned size)
+DFS::hash_scc (struct output_block *ob, unsigned first, unsigned size,
+	       bool ref_p, bool this_ref_p)
 {
   unsigned int last_classes = 0, iterations = 0;
 
@@ -1441,7 +1450,8 @@ DFS::hash_scc (struct output_block *ob,
 	      for (unsigned i = 0; i < size; ++i)
 		map.put (sccstack[first+i].t, sccstack[first+i].hash);
 
-	      DFS again (ob, sccstack[first+firstunique].t, false, false, true);
+	      DFS again (ob, sccstack[first+firstunique].t, ref_p, this_ref_p,
+			 true);
 	      gcc_assert (again.sccstack.length () == size);
 
 	      memcpy (sccstack.address () + first,
@@ -1539,9 +1549,9 @@ DFS::DFS_write_tree (struct output_block
 }
 
 
-/* Emit the physical representation of tree node EXPR to output block
-   OB.  If THIS_REF_P is true, the leaves of EXPR are emitted as references
-   via lto_output_tree_ref.  REF_P is used for streaming siblings of EXPR.  */
+/* Emit the physical representation of tree node EXPR to output block OB.
+   If THIS_REF_P is true, the leaves of EXPR are emitted as references via
+   lto_output_tree_ref.  REF_P is used for streaming siblings of EXPR.  */
 
 void
 lto_output_tree (struct output_block *ob, tree expr,

[-- Attachment #3: lto17.adb --]
[-- Type: text/x-adasrc, Size: 251 bytes --]

-- { dg-do compile }
-- { dg-options "-flto" }
-- { dg-require-effective-target lto }

package body Lto17 is

   function To_Chunk_List(C : Chunk) return Chunk_List is
   begin
      return new Chunk_List_Element'(C.Size, C, null);
   end;

end Lto17;

[-- Attachment #4: lto17.ads --]
[-- Type: text/x-adasrc, Size: 445 bytes --]

package Lto17 is

   type Chunk_List_Element;
   type Chunk_List is access Chunk_List_Element;

   type Arr is array (Natural range <>) of Integer;

   type Chunk(Size : Natural) is record
      Data  : Arr(1 .. Size);
      Where : Natural;
   end record;

   type Chunk_List_Element(Size : Natural) is record
      Chnk : Chunk(Size);
      Link : Chunk_List;
   end record;

   function To_Chunk_List(C : Chunk) return Chunk_List;

end Lto17;

Re: [patch] Fix ICE during SCC hashing in LTO mode

[Richard Biener]

On Tue, Jun 30, 2015 at 12:10 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> the attached Ada testcase triggers an internal error in LTO mode:
>
> eric@polaris:~/build/gcc/native> gcc/gnat1 -quiet lto17.adb -flto
> +===========================GNAT BUG DETECTED==============================+
> | 6.0.0 20150629 (experimental) [trunk revision 225111] (x86_64-suse-linux)
> GCC error:|
> | in hash_scc, at lto-streamer-out.c:1445
>
> This is the assertion verifying that you walk the same SCC using DFS after
> starting from another element of the SCC in hash_scc.  It fails here because
> this second DFS is not performed with the same THIS_REF_P parameter as the
> initial one, so there is no guaranteee that this will yield the same result
> because DFS_write_tree will behave differently.
>
> Fixed by passing the THIS_REF_P and REF_P parameters for FIRST to hash_scc.
>
> Tested on x86_64-suse-linux, OK for the mainline?

Ok (I suppose this also affects the GCC 5 branch?)

Thanks,
Richard.

>
> 2015-06-30  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * lto-streamer-out.c (class DFS): Adjust hash_scc method.
>         (DFS::DFS): Pass this_ref_p and ref_p to hash_scc.
>         (hash_scc): Add this_ref_p and ref_p parameters and pass them
>         to the inner DFS walk.
>
>
> 2015-06-30  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/lto17.ad[sb]: New test.
>
>
> --
> Eric Botcazou

Re: [patch] Fix ICE during SCC hashing in LTO mode

[Eric Botcazou]

> Ok (I suppose this also affects the GCC 5 branch?)

Thanks.  Yes, it does, but I'm not sure if we need to backport it immediately.

-- 
Eric Botcazou

Re: [patch] Fix ICE during SCC hashing in LTO mode

[Richard Biener]

On Tue, Jun 30, 2015 at 11:03 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Ok (I suppose this also affects the GCC 5 branch?)
>
> Thanks.  Yes, it does, but I'm not sure if we need to backport it immediately.

Given there is no PR to track this I'll count on you remembering to
backport it then ;)
If you'd want it in 5.2 then you'd need to backport it by this Friday morning.

Richard.

> --
> Eric Botcazou

Re: [patch] Fix ICE during SCC hashing in LTO mode

[Eric Botcazou]

> Given there is no PR to track this I'll count on you remembering to
> backport it then ;)

OK.

> If you'd want it in 5.2 then you'd need to backport it by this Friday
> morning.

No thanks. :-)

-- 
Eric Botcazou