diff mbox

[pph/trunk] Fix enum streaming

Message ID 20110530201351.GA2645@google.com
State New
Headers show

Commit Message

Diego Novillo May 30, 2011, 8:13 p.m. UTC
This fixes the issue I mentioned in
http://gcc.gnu.org/ml/gcc/2011-05/msg00325.html.  When enum
values are bigger than 255, lto_output_int_in_range was not
splitting up the values properly.

Additionally, when LTO_tags has more than 255 values (like it
does on the PPH) branch, output_record_start() will write 2 bytes
for every tag, but when writing out LTO_null we were writing only
1 byte because we'd call output_zero.  This caused the writer and
reader to be out of sync.

Tested on x86_64.  No regressions in the LTO testsuite. Still not
tested on trunk.  Richi, you still seem to be working in this
area.  Would you prefer a different fix?


Diego.


 	output_record_start with LTO_null instead of output_zero.
 	(lto_output_ts_binfo_tree_pointers): Likewise.
 	(lto_output_tree): Likewise.
 	(output_eh_try_list): Likewise.
 	(output_eh_region): Likewise.
 	(output_eh_lp): Likewise.
 	(output_eh_regions): Likewise.
 	(output_bb): Likewise.
 	(output_function): Likewise.
 	(output_unreferenced_globals): Likewise.
 	* lto-streamer.h (lto_output_int_in_range): Fix shift sign
 	when splitting values into byte-sized ranges.

---
 gcc/ChangeLog.pph      |   16 ++++++++++++++--
 gcc/lto-streamer-out.c |   28 ++++++++++++++--------------
 gcc/lto-streamer.h     |    6 +++---
 3 files changed, 31 insertions(+), 19 deletions(-)

Comments

Richard Biener May 30, 2011, 8:21 p.m. UTC | #1
On Mon, May 30, 2011 at 10:13 PM, Diego Novillo <dnovillo@google.com> wrote:
> This fixes the issue I mentioned in
> http://gcc.gnu.org/ml/gcc/2011-05/msg00325.html.  When enum
> values are bigger than 255, lto_output_int_in_range was not
> splitting up the values properly.
>
> Additionally, when LTO_tags has more than 255 values (like it
> does on the PPH) branch, output_record_start() will write 2 bytes
> for every tag, but when writing out LTO_null we were writing only
> 1 byte because we'd call output_zero.  This caused the writer and
> reader to be out of sync.
>
> Tested on x86_64.  No regressions in the LTO testsuite. Still not
> tested on trunk.  Richi, you still seem to be working in this
> area.  Would you prefer a different fix?

It's ok this way.

Thanks.
Richard.

>
> Diego.
>
>
>        output_record_start with LTO_null instead of output_zero.
>        (lto_output_ts_binfo_tree_pointers): Likewise.
>        (lto_output_tree): Likewise.
>        (output_eh_try_list): Likewise.
>        (output_eh_region): Likewise.
>        (output_eh_lp): Likewise.
>        (output_eh_regions): Likewise.
>        (output_bb): Likewise.
>        (output_function): Likewise.
>        (output_unreferenced_globals): Likewise.
>        * lto-streamer.h (lto_output_int_in_range): Fix shift sign
>        when splitting values into byte-sized ranges.
>
> ---
>  gcc/ChangeLog.pph      |   16 ++++++++++++++--
>  gcc/lto-streamer-out.c |   28 ++++++++++++++--------------
>  gcc/lto-streamer.h     |    6 +++---
>  3 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/gcc/ChangeLog.pph b/gcc/ChangeLog.pph
> index 3494a20..031e52c 100644
> --- a/gcc/ChangeLog.pph
> +++ b/gcc/ChangeLog.pph
> @@ -2,11 +2,23 @@
>
>        Merge from trunk rev 174363.
>
> -2011-05-30  Diego Novillo  <dnovillo@google.com>
> -
>        * lto-streamer.h (enum LTO_tags): Remove LTO_LAST_TAG.
>        Do not force the value on LTO_NUM_TAGS.
>
> +       * lto-streamer-out.c (lto_output_ts_decl_with_vis_tree_pointers): Call
> +       output_record_start with LTO_null instead of output_zero.
> +       (lto_output_ts_binfo_tree_pointers): Likewise.
> +       (lto_output_tree): Likewise.
> +       (output_eh_try_list): Likewise.
> +       (output_eh_region): Likewise.
> +       (output_eh_lp): Likewise.
> +       (output_eh_regions): Likewise.
> +       (output_bb): Likewise.
> +       (output_function): Likewise.
> +       (output_unreferenced_globals): Likewise.
> +       * lto-streamer.h (lto_output_int_in_range): Fix shift sign
> +       when splitting values into byte-sized ranges.
> +
>  2011-05-06  Diego Novillo  <dnovillo@google.com>
>
>        * lto-streamer-out.c (lto_output_tree): If the streamer
> diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
> index 21b82f6..64468f4 100644
> --- a/gcc/lto-streamer-out.c
> +++ b/gcc/lto-streamer-out.c
> @@ -917,7 +917,7 @@ lto_output_ts_decl_with_vis_tree_pointers (struct output_block *ob, tree expr,
>   if (DECL_ASSEMBLER_NAME_SET_P (expr))
>     lto_output_tree_or_ref (ob, DECL_ASSEMBLER_NAME (expr), ref_p);
>   else
> -    output_zero (ob);
> +    output_record_start (ob, LTO_null);
>
>   lto_output_tree_or_ref (ob, DECL_SECTION_NAME (expr), ref_p);
>   lto_output_tree_or_ref (ob, DECL_COMDAT_GROUP (expr), ref_p);
> @@ -1098,7 +1098,7 @@ lto_output_ts_binfo_tree_pointers (struct output_block *ob, tree expr,
>      is needed to build the empty BINFO node on the reader side.  */
>   FOR_EACH_VEC_ELT (tree, BINFO_BASE_BINFOS (expr), i, t)
>     lto_output_tree_or_ref (ob, t, ref_p);
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>
>   lto_output_tree_or_ref (ob, BINFO_OFFSET (expr), ref_p);
>   lto_output_tree_or_ref (ob, BINFO_VTABLE (expr), ref_p);
> @@ -1423,7 +1423,7 @@ lto_output_tree (struct output_block *ob, tree expr, bool ref_p)
>
>   if (expr == NULL_TREE)
>     {
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>       return;
>     }
>
> @@ -1498,7 +1498,7 @@ output_eh_try_list (struct output_block *ob, eh_catch first)
>       lto_output_tree_ref (ob, n->label);
>     }
>
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>  }
>
>
> @@ -1513,7 +1513,7 @@ output_eh_region (struct output_block *ob, eh_region r)
>
>   if (r == NULL)
>     {
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>       return;
>     }
>
> @@ -1576,7 +1576,7 @@ output_eh_lp (struct output_block *ob, eh_landing_pad lp)
>  {
>   if (lp == NULL)
>     {
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>       return;
>     }
>
> @@ -1645,9 +1645,9 @@ output_eh_regions (struct output_block *ob, struct function *fn)
>        }
>     }
>
> -  /* The 0 either terminates the record or indicates that there are no
> -     eh_records at all.  */
> -  output_zero (ob);
> +  /* The LTO_null either terminates the record or indicates that there
> +     are no eh_records at all.  */
> +  output_record_start (ob, LTO_null);
>  }
>
>
> @@ -1890,10 +1890,10 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
>              output_sleb128 (ob, region);
>            }
>          else
> -           output_zero (ob);
> +           output_record_start (ob, LTO_null);
>        }
>
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>
>       for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi))
>        {
> @@ -1906,7 +1906,7 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
>            output_phi (ob, phi);
>        }
>
> -      output_zero (ob);
> +      output_record_start (ob, LTO_null);
>     }
>  }
>
> @@ -2063,7 +2063,7 @@ output_function (struct cgraph_node *node)
>     output_bb (ob, bb, fn);
>
>   /* The terminator for this function.  */
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>
>   output_cfg (ob, fn);
>
> @@ -2177,7 +2177,7 @@ output_unreferenced_globals (cgraph_node_set set, varpool_node_set vset)
>       }
>   symbol_alias_set_destroy (defined);
>
> -  output_zero (ob);
> +  output_record_start (ob, LTO_null);
>
>   produce_asm (ob, NULL);
>   destroy_output_block (ob);
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index c06dcb4..2616252 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -1322,11 +1322,11 @@ lto_output_int_in_range (struct lto_output_stream *obs,
>   val -= min;
>   lto_output_1_stream (obs, val & 255);
>   if (range >= 0xff)
> -    lto_output_1_stream (obs, (val << 8) & 255);
> +    lto_output_1_stream (obs, (val >> 8) & 255);
>   if (range >= 0xffff)
> -    lto_output_1_stream (obs, (val << 16) & 255);
> +    lto_output_1_stream (obs, (val >> 16) & 255);
>   if (range >= 0xffffff)
> -    lto_output_1_stream (obs, (val << 24) & 255);
> +    lto_output_1_stream (obs, (val >> 24) & 255);
>  }
>
>  /* Input VAL into OBS and verify it is in range MIN...MAX that is supposed
> --
> 1.7.3.1
>
>
diff mbox

Patch

diff --git a/gcc/ChangeLog.pph b/gcc/ChangeLog.pph
index 3494a20..031e52c 100644
--- a/gcc/ChangeLog.pph
+++ b/gcc/ChangeLog.pph
@@ -2,11 +2,23 @@ 
 
 	Merge from trunk rev 174363.
 
-2011-05-30  Diego Novillo  <dnovillo@google.com>
-
 	* lto-streamer.h (enum LTO_tags): Remove LTO_LAST_TAG.
 	Do not force the value on LTO_NUM_TAGS.
 
+	* lto-streamer-out.c (lto_output_ts_decl_with_vis_tree_pointers): Call
+	output_record_start with LTO_null instead of output_zero.
+	(lto_output_ts_binfo_tree_pointers): Likewise.
+	(lto_output_tree): Likewise.
+	(output_eh_try_list): Likewise.
+	(output_eh_region): Likewise.
+	(output_eh_lp): Likewise.
+	(output_eh_regions): Likewise.
+	(output_bb): Likewise.
+	(output_function): Likewise.
+	(output_unreferenced_globals): Likewise.
+	* lto-streamer.h (lto_output_int_in_range): Fix shift sign
+	when splitting values into byte-sized ranges.
+
 2011-05-06  Diego Novillo  <dnovillo@google.com>
 
 	* lto-streamer-out.c (lto_output_tree): If the streamer
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 21b82f6..64468f4 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -917,7 +917,7 @@  lto_output_ts_decl_with_vis_tree_pointers (struct output_block *ob, tree expr,
   if (DECL_ASSEMBLER_NAME_SET_P (expr))
     lto_output_tree_or_ref (ob, DECL_ASSEMBLER_NAME (expr), ref_p);
   else
-    output_zero (ob);
+    output_record_start (ob, LTO_null);
 
   lto_output_tree_or_ref (ob, DECL_SECTION_NAME (expr), ref_p);
   lto_output_tree_or_ref (ob, DECL_COMDAT_GROUP (expr), ref_p);
@@ -1098,7 +1098,7 @@  lto_output_ts_binfo_tree_pointers (struct output_block *ob, tree expr,
      is needed to build the empty BINFO node on the reader side.  */
   FOR_EACH_VEC_ELT (tree, BINFO_BASE_BINFOS (expr), i, t)
     lto_output_tree_or_ref (ob, t, ref_p);
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 
   lto_output_tree_or_ref (ob, BINFO_OFFSET (expr), ref_p);
   lto_output_tree_or_ref (ob, BINFO_VTABLE (expr), ref_p);
@@ -1423,7 +1423,7 @@  lto_output_tree (struct output_block *ob, tree expr, bool ref_p)
 
   if (expr == NULL_TREE)
     {
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
       return;
     }
 
@@ -1498,7 +1498,7 @@  output_eh_try_list (struct output_block *ob, eh_catch first)
       lto_output_tree_ref (ob, n->label);
     }
 
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 }
 
 
@@ -1513,7 +1513,7 @@  output_eh_region (struct output_block *ob, eh_region r)
 
   if (r == NULL)
     {
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
       return;
     }
 
@@ -1576,7 +1576,7 @@  output_eh_lp (struct output_block *ob, eh_landing_pad lp)
 {
   if (lp == NULL)
     {
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
       return;
     }
 
@@ -1645,9 +1645,9 @@  output_eh_regions (struct output_block *ob, struct function *fn)
 	}
     }
 
-  /* The 0 either terminates the record or indicates that there are no
-     eh_records at all.  */
-  output_zero (ob);
+  /* The LTO_null either terminates the record or indicates that there
+     are no eh_records at all.  */
+  output_record_start (ob, LTO_null);
 }
 
 
@@ -1890,10 +1890,10 @@  output_bb (struct output_block *ob, basic_block bb, struct function *fn)
 	      output_sleb128 (ob, region);
 	    }
 	  else
-	    output_zero (ob);
+	    output_record_start (ob, LTO_null);
 	}
 
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
 
       for (bsi = gsi_start_phis (bb); !gsi_end_p (bsi); gsi_next (&bsi))
 	{
@@ -1906,7 +1906,7 @@  output_bb (struct output_block *ob, basic_block bb, struct function *fn)
 	    output_phi (ob, phi);
 	}
 
-      output_zero (ob);
+      output_record_start (ob, LTO_null);
     }
 }
 
@@ -2063,7 +2063,7 @@  output_function (struct cgraph_node *node)
     output_bb (ob, bb, fn);
 
   /* The terminator for this function.  */
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 
   output_cfg (ob, fn);
 
@@ -2177,7 +2177,7 @@  output_unreferenced_globals (cgraph_node_set set, varpool_node_set vset)
       }
   symbol_alias_set_destroy (defined);
 
-  output_zero (ob);
+  output_record_start (ob, LTO_null);
 
   produce_asm (ob, NULL);
   destroy_output_block (ob);
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index c06dcb4..2616252 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -1322,11 +1322,11 @@  lto_output_int_in_range (struct lto_output_stream *obs,
   val -= min;
   lto_output_1_stream (obs, val & 255);
   if (range >= 0xff)
-    lto_output_1_stream (obs, (val << 8) & 255);
+    lto_output_1_stream (obs, (val >> 8) & 255);
   if (range >= 0xffff)
-    lto_output_1_stream (obs, (val << 16) & 255);
+    lto_output_1_stream (obs, (val >> 16) & 255);
   if (range >= 0xffffff)
-    lto_output_1_stream (obs, (val << 24) & 255);
+    lto_output_1_stream (obs, (val >> 24) & 255);
 }
 
 /* Input VAL into OBS and verify it is in range MIN...MAX that is supposed