Re: [LTO][PATCH] Fix missing tree flags bug.

[Diego Novillo <dnovillo at google dot com>]

Doug Kwan (Ãö®¶¼w) wrote:

>     Could someone review and approve this patch? It fixes a problem
> where the first 5 bits of tree flags of a VAR_DECL node got dropped on
> host that has 32-bit HOST_WIDE_INT.  I tested it on i686-linux-gnu.

Committed with a couple of minor formatting fixes and added an extra 
compile-time check for the width of BITS_PER_LTO_FLAGS_TYPES.



Diego.
2007-11-13  Diego Novillo  <dnovillo@google.com>

	* lto-tags.h: Call #error if BITS_PER_LTO_FLAGS_TYPE is
	not wide enough.

2007-11-13  Doug Kwan  <dougkwan@google.com>

	* lto-function-out.c (output_widest_uint_uleb128_stream,
	output_widest_uint_uleb128): New functions. 
	(output_tree_flags): Use lto_flags_type instead of unsigned
	HOST_WIDE_INT.  Call output_widest_unint_uleb128 instead of
	output_uleb128.
	(lto_debug_tree_flags): Replace HOST_BITS_PER_WIDE_INT with
	BITS_PER_LTO_FLAGS_TYPE as appropriate.
	* lto-tags.h (lto_flags_type): New type.
	(BITS_PER_LTO_FLAGS_TYPES): New macro.
	(lto_debug_tree_flags): Change prototype to use
	lto_flags_type.

lto/ChangeLog

	* lto-read.c (input_widest_uint_uleb128): New function.
	(input_tree_flags, process_tree_flags, input_line_info,
	input_expr_operand, input_local_var, input_phi, input_ssa_names):
	Change to use lto_flags_type and BITS_PER_LTO_FLAGS_TYPES instead of
	unsigned HOST_WIDE_INT and HOST_BITS_PER_WIDE_INT.
	(lto_static_init_local): Add code to assert that lto_flags_type is
	wide enough.

Index: lto-function-out.c
===================================================================
--- lto-function-out.c	(revision 130144)
+++ lto-function-out.c	(working copy)
@@ -558,7 +558,29 @@ output_uleb128_stream (struct output_str
       output_1_stream (obs, byte);
     }
   while (work != 0);
+}
+
+/* Identical to output_uleb128_stream above except using unsigned 
+   HOST_WIDEST_INT type.  For efficiency on host where unsigned HOST_WIDEST_INT
+   is not native, we only use this if we know that HOST_WIDE_INT is not wide
+   enough.  */
 
+static void
+output_widest_uint_uleb128_stream (struct output_stream *obs,
+				   unsigned HOST_WIDEST_INT work)
+{
+  LTO_DEBUG_WIDE ("U", work);
+  do
+    {
+      unsigned int byte = (work & 0x7f);
+      work >>= 7;
+      if (work != 0)
+	/* More bytes to follow.  */
+	byte |= 0x80;
+
+      output_1_stream (obs, byte);
+    }
+  while (work != 0);
 }
 
 
@@ -570,6 +592,15 @@ output_uleb128 (struct output_block *ob,
   output_uleb128_stream (ob->main_stream, work);
 }
 
+/* HOST_WIDEST_INT version of output_uleb128.  OB and WORK are as in
+   output_uleb128. */
+
+static void
+output_widest_uint_uleb128 (struct output_block *ob,
+			    unsigned HOST_WIDEST_INT work)
+{
+  output_widest_uint_uleb128_stream (ob->main_stream, work);
+}
 
 /* Output a signed LEB128 quantity.  */
 
@@ -761,7 +792,7 @@ output_tree_flags (struct output_block *
 		   tree expr, 
 		   bool force_loc)
 {
-  unsigned HOST_WIDE_INT flags = 0;
+  lto_flags_type flags = 0;
   const char *file_to_write = NULL;
   int line_to_write = -1;
 
@@ -890,7 +921,7 @@ output_tree_flags (struct output_block *
 	}
 
       LTO_DEBUG_TOKEN ("flags");
-      output_uleb128 (ob, flags);
+      output_widest_uint_uleb128 (ob, flags);
       LTO_DEBUG_TREE_FLAGS (code, flags);
 
       if (file_to_write)
@@ -2389,9 +2420,9 @@ debug_out_fun (struct lto_debug_context 
 void 
 lto_debug_tree_flags (struct lto_debug_context *context, 
 		      enum tree_code code, 
-		      unsigned HOST_WIDE_INT flags)
+		      lto_flags_type flags)
 {
-#define CLEAROUT (HOST_BITS_PER_WIDE_INT - 1)
+#define CLEAROUT (BITS_PER_LTO_FLAGS_TYPE - 1)
 
 #define START_CLASS_SWITCH()              \
   {                                       \
@@ -2420,7 +2451,7 @@ lto_debug_tree_flags (struct lto_debug_c
 #define ADD_VIS_FLAG(flag_name)  \
   { if (flags >> CLEAROUT) lto_debug_token (context, " " # flag_name ); flags <<= 1; }
 #define ADD_VIS_FLAG_SIZE(flag_name,size)					\
-  { if (flags >> (HOST_BITS_PER_WIDE_INT - size)) lto_debug_token (context, " " # flag_name ); flags <<= size; }
+  { if (flags >> (BITS_PER_LTO_FLAGS_TYPE - size)) lto_debug_token (context, " " # flag_name ); flags <<= size; }
 #define END_EXPR_CASE(class)      break;
 #define END_EXPR_SWITCH()                 \
     default:                              \
Index: lto-tags.h
===================================================================
--- lto-tags.h	(revision 130144)
+++ lto-tags.h	(working copy)
@@ -526,6 +526,14 @@ struct lto_debug_context
   void * main_data;
 };
 
+/* The VAR_DECL tree code has more than 32 bits in flags.  On some hosts,
+   HOST_WIDE_INT is not wide enough.  */
+typedef unsigned HOST_WIDEST_INT 	lto_flags_type;
+#define	BITS_PER_LTO_FLAGS_TYPE		HOST_BITS_PER_WIDEST_INT
+
+#if BITS_PER_LTO_FLAGS_TYPE <= 32
+#  error "Your host should support integer types wider than 32 bits."
+#endif
 
 /* The serialization plan is that when any of the current file, line,
    or col change (from the state last serialized), we write the
@@ -551,7 +559,7 @@ extern void lto_debug_indent_token (stru
 extern void lto_debug_integer (struct lto_debug_context *, const char *, HOST_WIDE_INT, HOST_WIDE_INT);
 extern void lto_debug_string (struct lto_debug_context *, const char *, int);
 extern void lto_debug_token (struct lto_debug_context *, const char *);
-extern void lto_debug_tree_flags (struct lto_debug_context *, enum tree_code, unsigned HOST_WIDE_INT);
+extern void lto_debug_tree_flags (struct lto_debug_context *, enum tree_code, lto_flags_type);
 extern void lto_debug_undent (struct lto_debug_context *);
 extern void lto_debug_wide (struct lto_debug_context *, const char *, HOST_WIDE_INT);
 
Index: lto/lto-read.c
===================================================================
--- lto/lto-read.c	(revision 130144)
+++ lto/lto-read.c	(working copy)
@@ -178,6 +178,27 @@ input_uleb128 (struct input_block *ib)
     }
 }
 
+/* HOST_WIDEST_INT version of input_uleb128.  IB is as in input_uleb128.  */
+
+static unsigned HOST_WIDEST_INT 
+input_widest_uint_uleb128 (struct input_block *ib)
+{
+  unsigned HOST_WIDEST_INT result = 0;
+  int shift = 0;
+  unsigned HOST_WIDEST_INT byte;
+
+  while (true)
+    {
+      byte = input_1_unsigned (ib);
+      result |= (byte & 0x7f) << shift;
+      shift += 7;
+      if ((byte & 0x80) == 0)
+	{
+	  LTO_DEBUG_WIDE ("U", result);
+	  return result;
+	}
+    }
+}
 
 /* Read an SLEB128 Number of IB.  */
 
@@ -352,20 +373,20 @@ get_type_ref (struct data_in *data_in, s
 }
 
 /* Set all of the FLAGS for NODE.  */
-#define CLEAROUT (HOST_BITS_PER_WIDE_INT - 1)
+#define CLEAROUT (BITS_PER_LTO_FLAGS_TYPE - 1)
 
 
 /* Read the tree flags for CODE from IB.  */
 
-static unsigned HOST_WIDE_INT 
+static lto_flags_type
 input_tree_flags (struct input_block *ib, enum tree_code code, bool force)
 {
-  unsigned HOST_WIDE_INT flags;
+  lto_flags_type flags;
 
   if (force || TEST_BIT (lto_flags_needed_for, code))
     {
       LTO_DEBUG_TOKEN ("flags");
-      flags = input_uleb128 (ib);
+      flags = input_widest_uint_uleb128 (ib);
       LTO_DEBUG_TREE_FLAGS (code, flags);
     }
   else
@@ -377,12 +398,12 @@ input_tree_flags (struct input_block *ib
 /* Set all of the flag bits inside EXPR by unpacking FLAGS.  */
 
 static void
-process_tree_flags (tree expr, unsigned HOST_WIDE_INT flags)
+process_tree_flags (tree expr, lto_flags_type flags)
 {
   enum tree_code code = TREE_CODE (expr);
   /* Shift the flags up so that the first flag is at the top of the
      flag word.  */
-  flags <<= HOST_BITS_PER_WIDE_INT - flags_length_for_code[code];
+  flags <<= BITS_PER_LTO_FLAGS_TYPE - flags_length_for_code[code];
 
 #define START_CLASS_SWITCH()              \
   {                                       \
@@ -411,7 +432,7 @@ process_tree_flags (tree expr, unsigned 
 #define ADD_VIS_FLAG(flag_name)  \
   { expr->decl_with_vis. flag_name = (flags >> CLEAROUT); flags <<= 1; }
 #define ADD_VIS_FLAG_SIZE(flag_name,size)					\
-  { expr->decl_with_vis. flag_name = (flags >> (HOST_BITS_PER_WIDE_INT - size)); flags <<= size; }
+  { expr->decl_with_vis. flag_name = (flags >> (BITS_PER_LTO_FLAGS_TYPE - size)); flags <<= size; }
 #define END_EXPR_CASE(class)      break;
 #define END_EXPR_SWITCH()                 \
     default:                              \
@@ -473,7 +494,7 @@ canon_file_name (const char *string)
 
 static void 
 input_line_info (struct input_block *ib, struct data_in *data_in, 
-		 unsigned HOST_WIDE_INT flags)
+		 lto_flags_type flags)
 {
 #ifdef USE_MAPPED_LOCATION  
   if (flags & LTO_SOURCE_FILE)
@@ -576,7 +597,7 @@ input_expr_operand (struct input_block *
 {
   enum tree_code code = tag_to_expr[tag];
   tree type = NULL_TREE;
-  unsigned HOST_WIDE_INT flags;
+  lto_flags_type flags;
   gcc_assert (code);
   tree result = NULL_TREE;
   
@@ -1233,7 +1254,7 @@ input_local_var (struct input_block *ib,
   unsigned int name_index;
   tree name;
   tree type;
-  unsigned HOST_WIDE_INT flags;
+  lto_flags_type flags;
   tree result;
   tree context;
 
@@ -1470,7 +1491,7 @@ static tree
 input_phi (struct input_block *ib, basic_block bb, 
 	   struct data_in *data_in, struct function *fn)
 {
-  unsigned HOST_WIDE_INT flags = input_tree_flags (ib, PHI_NODE, false);
+  lto_flags_type flags = input_tree_flags (ib, PHI_NODE, false);
 
   tree phi_result = VEC_index (tree, SSANAMES (fn), input_uleb128 (ib));
   int len = EDGE_COUNT (bb->preds);
@@ -1523,7 +1544,7 @@ input_ssa_names (struct input_block *ib,
     {
       tree ssa_name;
       tree name;
-      unsigned HOST_WIDE_INT flags;
+      lto_flags_type flags;
 
       /* Skip over the elements that had been freed.  */
       while (VEC_length (tree, SSANAMES (fn)) < i)
@@ -1726,6 +1747,13 @@ lto_static_init_local (void)
 #undef END_EXPR_CASE
 #undef END_EXPR_SWITCH
 
+  /* Verify that lto_flags_type is wide enough.  */
+  {
+    int code;
+    for (code = 0; code < NUM_TREE_CODES; code++)
+      gcc_assert (flags_length_for_code[code] <= BITS_PER_LTO_FLAGS_TYPE);
+  }
+  
   lto_static_init ();
   tree_register_cfg_hooks ();