[PATCH] Remove dead code from c_common_get_alias_set

[PATCH] Remove dead code from c_common_get_alias_set

[Bernd Edlinger]

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

Hi!


This removes some dead and unreachable code in c_common_get_alias_set:
Because cc1 was recently changed to be only called with one file at a
time, the code after "if (num_in_fnames == 1) return -1;" is no longer
reachable, and can thus be removed.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-get-alias-set-remove-dead-code.diff --]
[-- Type: text/x-patch; name="patch-get-alias-set-remove-dead-code.diff", Size: 4384 bytes --]

2017-04-21  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (c_type_hasher): Remove.
	(type_hash_table): Remove.
	(c_common_get_alias_set): Remove unreachable code.

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(Revision 247029)
+++ gcc/c-family/c-common.c	(Arbeitskopie)
@@ -3508,67 +3508,6 @@ c_apply_type_quals_to_decl (int type_quals, tree d
     }
 }
 
-struct c_type_hasher : ggc_ptr_hash<tree_node>
-{
-  static hashval_t hash (tree);
-  static bool equal (tree, tree);
-};
-
-/* Hash function for the problem of multiple type definitions in
-   different files.  This must hash all types that will compare
-   equal via comptypes to the same value.  In practice it hashes
-   on some of the simple stuff and leaves the details to comptypes.  */
-
-hashval_t
-c_type_hasher::hash (tree t)
-{
-  int n_elements;
-  int shift, size;
-  tree t2;
-  switch (TREE_CODE (t))
-    {
-    /* For pointers, hash on pointee type plus some swizzling.  */
-    case POINTER_TYPE:
-      return hash (TREE_TYPE (t)) ^ 0x3003003;
-    /* Hash on number of elements and total size.  */
-    case ENUMERAL_TYPE:
-      shift = 3;
-      t2 = TYPE_VALUES (t);
-      break;
-    case RECORD_TYPE:
-      shift = 0;
-      t2 = TYPE_FIELDS (t);
-      break;
-    case QUAL_UNION_TYPE:
-      shift = 1;
-      t2 = TYPE_FIELDS (t);
-      break;
-    case UNION_TYPE:
-      shift = 2;
-      t2 = TYPE_FIELDS (t);
-      break;
-    default:
-      gcc_unreachable ();
-    }
-  /* FIXME: We want to use a DECL_CHAIN iteration method here, but
-     TYPE_VALUES of ENUMERAL_TYPEs is stored as a TREE_LIST.  */
-  n_elements = list_length (t2);
-  /* We might have a VLA here.  */
-  if (TREE_CODE (TYPE_SIZE (t)) != INTEGER_CST)
-    size = 0;
-  else
-    size = TREE_INT_CST_LOW (TYPE_SIZE (t));
-  return ((size << 24) | (n_elements << shift));
-}
-
-bool
-c_type_hasher::equal (tree t1, tree t2)
-{
-  return lang_hooks.types_compatible_p (t1, t2);
-}
-
-static GTY(()) hash_table<c_type_hasher> *type_hash_table;
-
 /* Return the typed-based alias set for T, which may be an expression
    or a type.  Return -1 if we don't do anything special.  */
 
@@ -3607,60 +3546,6 @@ c_common_get_alias_set (tree t)
 	return get_alias_set (t1);
     }
 
-  /* Handle the case of multiple type nodes referring to "the same" type,
-     which occurs with IMA.  These share an alias set.  FIXME:  Currently only
-     C90 is handled.  (In C99 type compatibility is not transitive, which
-     complicates things mightily. The alias set splay trees can theoretically
-     represent this, but insertion is tricky when you consider all the
-     different orders things might arrive in.) */
-
-  if (c_language != clk_c || flag_isoc99)
-    return -1;
-
-  /* Save time if there's only one input file.  */
-  if (num_in_fnames == 1)
-    return -1;
-
-  /* Pointers need special handling if they point to any type that
-     needs special handling (below).  */
-  if (TREE_CODE (t) == POINTER_TYPE)
-    {
-      tree t2;
-      /* Find bottom type under any nested POINTERs.  */
-      for (t2 = TREE_TYPE (t);
-	   TREE_CODE (t2) == POINTER_TYPE;
-	   t2 = TREE_TYPE (t2))
-	;
-      if (!RECORD_OR_UNION_TYPE_P (t2)
-	  && TREE_CODE (t2) != ENUMERAL_TYPE)
-	return -1;
-      if (TYPE_SIZE (t2) == 0)
-	return -1;
-    }
-  /* These are the only cases that need special handling.  */
-  if (!RECORD_OR_UNION_TYPE_P (t)
-      && TREE_CODE (t) != ENUMERAL_TYPE
-      && TREE_CODE (t) != POINTER_TYPE)
-    return -1;
-  /* Undefined? */
-  if (TYPE_SIZE (t) == 0)
-    return -1;
-
-  /* Look up t in hash table.  Only one of the compatible types within each
-     alias set is recorded in the table.  */
-  if (!type_hash_table)
-    type_hash_table = hash_table<c_type_hasher>::create_ggc (1021);
-  tree *slot = type_hash_table->find_slot (t, INSERT);
-  if (*slot != NULL)
-    {
-      TYPE_ALIAS_SET (t) = TYPE_ALIAS_SET ((tree)*slot);
-      return TYPE_ALIAS_SET ((tree)*slot);
-    }
-  else
-    /* Our caller will assign and record (in t) a new alias set; all we need
-       to do is remember t in the hash table.  */
-    *slot = t;
-
   return -1;
 }
 \f

Re: [PATCH] Remove dead code from c_common_get_alias_set

[Richard Biener]

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

On Fri, 21 Apr 2017, Bernd Edlinger wrote:

> Hi!
> 
> 
> This removes some dead and unreachable code in c_common_get_alias_set:
> Because cc1 was recently changed to be only called with one file at a
> time, the code after "if (num_in_fnames == 1) return -1;" is no longer
> reachable, and can thus be removed.

While I think you are correct it looks like c_common_parse_file still
happily parses multiple infiles.  That is, only for
flag_preprocess_only we have a

      if (num_in_fnames > 1)
        error ("too many filenames given.  Type %s --help for usage",
               progname);

and:

gcc> ./cc1 -quiet t.c t2.c
t2.c:5:6: error: conflicting types for ‘bar’
 void bar () { struct X x; *(volatile char *)x.buf = 1; }
      ^~~
t.c:8:1: note: previous definition of ‘bar’ was here
 bar (int x)
 ^~~

which means it actually still "works" to combine two source files
(yes, the driver no longer seems to have the ability to pass down
multiple inputs to cc1).

Thus, can you first remove that "feature"?

Thanks,
Richard.

> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Remove dead code from c_common_get_alias_set

[Bernd Edlinger]

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

On 04/24/17 09:00, Richard Biener wrote:
> On Fri, 21 Apr 2017, Bernd Edlinger wrote:
>
>> Hi!
>>
>>
>> This removes some dead and unreachable code in c_common_get_alias_set:
>> Because cc1 was recently changed to be only called with one file at a
>> time, the code after "if (num_in_fnames == 1) return -1;" is no longer
>> reachable, and can thus be removed.
>
> While I think you are correct it looks like c_common_parse_file still
> happily parses multiple infiles.  That is, only for
> flag_preprocess_only we have a
>
>       if (num_in_fnames > 1)
>         error ("too many filenames given.  Type %s --help for usage",
>                progname);
>
> and:
>
> gcc> ./cc1 -quiet t.c t2.c
> t2.c:5:6: error: conflicting types for ‘bar’
>  void bar () { struct X x; *(volatile char *)x.buf = 1; }
>       ^~~
> t.c:8:1: note: previous definition of ‘bar’ was here
>  bar (int x)
>  ^~~
>
> which means it actually still "works" to combine two source files
> (yes, the driver no longer seems to have the ability to pass down
> multiple inputs to cc1).
>
> Thus, can you first remove that "feature"?
>

Yes, sure.  See updated patch.


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-get-alias-set-remove-dead-code.diff --]
[-- Type: text/x-patch; name="patch-get-alias-set-remove-dead-code.diff", Size: 5216 bytes --]

2017-04-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* c-common.c (c_type_hasher, type_hash_table): Remove.
	(c_common_get_alias_set): Remove unreachable code.
	* c-opts.c (c_common_post_options): Make sure cc1 takes only one file.

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(Revision 247029)
+++ gcc/c-family/c-common.c	(Arbeitskopie)
@@ -3508,67 +3508,6 @@ c_apply_type_quals_to_decl (int type_quals, tree d
     }
 }
 
-struct c_type_hasher : ggc_ptr_hash<tree_node>
-{
-  static hashval_t hash (tree);
-  static bool equal (tree, tree);
-};
-
-/* Hash function for the problem of multiple type definitions in
-   different files.  This must hash all types that will compare
-   equal via comptypes to the same value.  In practice it hashes
-   on some of the simple stuff and leaves the details to comptypes.  */
-
-hashval_t
-c_type_hasher::hash (tree t)
-{
-  int n_elements;
-  int shift, size;
-  tree t2;
-  switch (TREE_CODE (t))
-    {
-    /* For pointers, hash on pointee type plus some swizzling.  */
-    case POINTER_TYPE:
-      return hash (TREE_TYPE (t)) ^ 0x3003003;
-    /* Hash on number of elements and total size.  */
-    case ENUMERAL_TYPE:
-      shift = 3;
-      t2 = TYPE_VALUES (t);
-      break;
-    case RECORD_TYPE:
-      shift = 0;
-      t2 = TYPE_FIELDS (t);
-      break;
-    case QUAL_UNION_TYPE:
-      shift = 1;
-      t2 = TYPE_FIELDS (t);
-      break;
-    case UNION_TYPE:
-      shift = 2;
-      t2 = TYPE_FIELDS (t);
-      break;
-    default:
-      gcc_unreachable ();
-    }
-  /* FIXME: We want to use a DECL_CHAIN iteration method here, but
-     TYPE_VALUES of ENUMERAL_TYPEs is stored as a TREE_LIST.  */
-  n_elements = list_length (t2);
-  /* We might have a VLA here.  */
-  if (TREE_CODE (TYPE_SIZE (t)) != INTEGER_CST)
-    size = 0;
-  else
-    size = TREE_INT_CST_LOW (TYPE_SIZE (t));
-  return ((size << 24) | (n_elements << shift));
-}
-
-bool
-c_type_hasher::equal (tree t1, tree t2)
-{
-  return lang_hooks.types_compatible_p (t1, t2);
-}
-
-static GTY(()) hash_table<c_type_hasher> *type_hash_table;
-
 /* Return the typed-based alias set for T, which may be an expression
    or a type.  Return -1 if we don't do anything special.  */
 
@@ -3607,60 +3546,6 @@ c_common_get_alias_set (tree t)
 	return get_alias_set (t1);
     }
 
-  /* Handle the case of multiple type nodes referring to "the same" type,
-     which occurs with IMA.  These share an alias set.  FIXME:  Currently only
-     C90 is handled.  (In C99 type compatibility is not transitive, which
-     complicates things mightily. The alias set splay trees can theoretically
-     represent this, but insertion is tricky when you consider all the
-     different orders things might arrive in.) */
-
-  if (c_language != clk_c || flag_isoc99)
-    return -1;
-
-  /* Save time if there's only one input file.  */
-  if (num_in_fnames == 1)
-    return -1;
-
-  /* Pointers need special handling if they point to any type that
-     needs special handling (below).  */
-  if (TREE_CODE (t) == POINTER_TYPE)
-    {
-      tree t2;
-      /* Find bottom type under any nested POINTERs.  */
-      for (t2 = TREE_TYPE (t);
-	   TREE_CODE (t2) == POINTER_TYPE;
-	   t2 = TREE_TYPE (t2))
-	;
-      if (!RECORD_OR_UNION_TYPE_P (t2)
-	  && TREE_CODE (t2) != ENUMERAL_TYPE)
-	return -1;
-      if (TYPE_SIZE (t2) == 0)
-	return -1;
-    }
-  /* These are the only cases that need special handling.  */
-  if (!RECORD_OR_UNION_TYPE_P (t)
-      && TREE_CODE (t) != ENUMERAL_TYPE
-      && TREE_CODE (t) != POINTER_TYPE)
-    return -1;
-  /* Undefined? */
-  if (TYPE_SIZE (t) == 0)
-    return -1;
-
-  /* Look up t in hash table.  Only one of the compatible types within each
-     alias set is recorded in the table.  */
-  if (!type_hash_table)
-    type_hash_table = hash_table<c_type_hasher>::create_ggc (1021);
-  tree *slot = type_hash_table->find_slot (t, INSERT);
-  if (*slot != NULL)
-    {
-      TYPE_ALIAS_SET (t) = TYPE_ALIAS_SET ((tree)*slot);
-      return TYPE_ALIAS_SET ((tree)*slot);
-    }
-  else
-    /* Our caller will assign and record (in t) a new alias set; all we need
-       to do is remember t in the hash table.  */
-    *slot = t;
-
   return -1;
 }
 \f
Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(Revision 247104)
+++ gcc/c-family/c-opts.c	(Arbeitskopie)
@@ -971,6 +971,10 @@ c_common_post_options (const char **pfilename)
 #endif
     }
 
+  if (num_in_fnames > 1)
+    error ("too many filenames given.  Type %s --help for usage",
+	   progname);
+
   if (flag_preprocess_only)
     {
       /* Open the output now.  We must do so even if flag_no_output is
@@ -987,10 +991,6 @@ c_common_post_options (const char **pfilename)
 	  return false;
 	}
 
-      if (num_in_fnames > 1)
-	error ("too many filenames given.  Type %s --help for usage",
-	       progname);
-
       init_pp_output (out_stream);
     }
   else

Re: [PATCH] Remove dead code from c_common_get_alias_set

[Richard Biener]

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

On Mon, 24 Apr 2017, Bernd Edlinger wrote:

> On 04/24/17 09:00, Richard Biener wrote:
> > On Fri, 21 Apr 2017, Bernd Edlinger wrote:
> >
> >> Hi!
> >>
> >>
> >> This removes some dead and unreachable code in c_common_get_alias_set:
> >> Because cc1 was recently changed to be only called with one file at a
> >> time, the code after "if (num_in_fnames == 1) return -1;" is no longer
> >> reachable, and can thus be removed.
> >
> > While I think you are correct it looks like c_common_parse_file still
> > happily parses multiple infiles.  That is, only for
> > flag_preprocess_only we have a
> >
> >       if (num_in_fnames > 1)
> >         error ("too many filenames given.  Type %s --help for usage",
> >                progname);
> >
> > and:
> >
> > gcc> ./cc1 -quiet t.c t2.c
> > t2.c:5:6: error: conflicting types for ‘bar’
> >  void bar () { struct X x; *(volatile char *)x.buf = 1; }
> >       ^~~
> > t.c:8:1: note: previous definition of ‘bar’ was here
> >  bar (int x)
> >  ^~~
> >
> > which means it actually still "works" to combine two source files
> > (yes, the driver no longer seems to have the ability to pass down
> > multiple inputs to cc1).
> >
> > Thus, can you first remove that "feature"?
> >
> 
> Yes, sure.  See updated patch.

Ok.

Thanks,
Richard.