c++, symtab: Support &typeid(x) == &typeid(y) in constant evaluation [PR103600]

Message ID 20211209201523.GK2646553@tucnak
State Committed
Headers
Series c++, symtab: Support &typeid(x) == &typeid(y) in constant evaluation [PR103600] |

Commit Message

Jakub Jelinek Dec. 9, 2021, 8:15 p.m. UTC
  On Wed, Dec 08, 2021 at 08:53:03AM -0500, Jason Merrill wrote:
> > As mentioned in the PR, the varpool/middle-end code is trying to be
> > conservative with address comparisons of different vars if those vars
> > don't bind locally, because of possible aliases in other TUs etc.
> 
> Would it make sense to assume that DECL_ARTIFICIAL variables can't be
> aliases?  If not, could we have some way of marking a variable as
> non-aliasing, perhaps an attribute?

I think DECL_ARTIFICIAL vars generally can overlap.

The following patch adds a GCC internal attribute "non overlapping"
and uses it in symtab_node::equal_address_to.
Not sure what plans has Honza in that area and whether it would be useful
to make the attribute public and let users assert that some variable will
never overlap with other variables, won't have aliases etc.

> During constant evaluation, the operator== could compare the type_info
> address instead of the __name address, reducing this to the previous
> problem.

Ah, indeed, good idea.  FYI, clang++ seems to constant fold
&typeid(x) != &typeid(y) already, so Jonathan could use it even for
clang++ in the constexpr operator==.  But it folds even
extern int &a, &b;
constexpr bool c = &a != &b;
regardless of whether some other TU has
int a;
int b __attribute__((alias (a));
or not.

Here is an updated patch, ok for trunk if it passes bootstrap/regtest?

2021-12-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/103600
gcc/
	* symtab.c (symtab_node::equal_address_to): Return 0 if one of
	VAR_DECLs has "non overlapping" attribute and rs1 != rs2.
gcc/c-family/
	* c-attribs.c (handle_non_overlapping_attribute): New function.
	(c_common_attribute_table): Add "non overlapping" attribute.
gcc/cp/
	* rtti.c (get_tinfo_decl_direct): Add "non overlapping" attribute
	to DECL_TINFO_P VAR_DECLs.
gcc/testsuite/
	* g++.dg/cpp0x/constexpr-typeid2.C: New test.



	Jakub
  

Comments

Jan Hubicka Dec. 9, 2021, 8:41 p.m. UTC | #1
> 
> Ah, indeed, good idea.  FYI, clang++ seems to constant fold
> &typeid(x) != &typeid(y) already, so Jonathan could use it even for
> clang++ in the constexpr operator==.  But it folds even
> extern int &a, &b;
> constexpr bool c = &a != &b;
> regardless of whether some other TU has
> int a;
> int b __attribute__((alias (a));
> or not.
> 
> Here is an updated patch, ok for trunk if it passes bootstrap/regtest?

Looks good to me.  We are somewhat inconsistent on when we support
overleap and when we don't.  Also we produce local symbols that can be
later globalized by partitioning.  So perhaps we want this to eventually
become flag in symtab node and unify the logic in aliasing code etc, but
that can wait for next stage1.

Honza
  
Jason Merrill Dec. 9, 2021, 11:09 p.m. UTC | #2
On 12/9/21 15:15, Jakub Jelinek wrote:
> On Wed, Dec 08, 2021 at 08:53:03AM -0500, Jason Merrill wrote:
>>> As mentioned in the PR, the varpool/middle-end code is trying to be
>>> conservative with address comparisons of different vars if those vars
>>> don't bind locally, because of possible aliases in other TUs etc.
>>
>> Would it make sense to assume that DECL_ARTIFICIAL variables can't be
>> aliases?  If not, could we have some way of marking a variable as
>> non-aliasing, perhaps an attribute?
> 
> I think DECL_ARTIFICIAL vars generally can overlap.
> 
> The following patch adds a GCC internal attribute "non overlapping"
> and uses it in symtab_node::equal_address_to.
> Not sure what plans has Honza in that area and whether it would be useful
> to make the attribute public and let users assert that some variable will
> never overlap with other variables, won't have aliases etc.
> 
>> During constant evaluation, the operator== could compare the type_info
>> address instead of the __name address, reducing this to the previous
>> problem.
> 
> Ah, indeed, good idea.  FYI, clang++ seems to constant fold
> &typeid(x) != &typeid(y) already, so Jonathan could use it even for
> clang++ in the constexpr operator==.  But it folds even
> extern int &a, &b;
> constexpr bool c = &a != &b;
> regardless of whether some other TU has
> int a;
> int b __attribute__((alias (a));
> or not.
> 
> Here is an updated patch, ok for trunk if it passes bootstrap/regtest?

LGTM for fixing the specific typeid issue, if Honza has no objection.

For the more general comparison of decls like your a != b example above 
I think clang is in the right; in manifestly constant-evaluated context 
(folding_initializer) we should return that they are unequal and prevent 
a later alias declaration, like we do for comparison to 0 in 
maybe_nonzero_address.  It's possible that this gives a wrong answer 
based on something in another translation unit, but that's unlikely, and 
taking that chance seems better than rejecting code that needs a 
constant answer.

> 2021-12-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/103600
> gcc/
> 	* symtab.c (symtab_node::equal_address_to): Return 0 if one of
> 	VAR_DECLs has "non overlapping" attribute and rs1 != rs2.
> gcc/c-family/
> 	* c-attribs.c (handle_non_overlapping_attribute): New function.
> 	(c_common_attribute_table): Add "non overlapping" attribute.
> gcc/cp/
> 	* rtti.c (get_tinfo_decl_direct): Add "non overlapping" attribute
> 	to DECL_TINFO_P VAR_DECLs.
> gcc/testsuite/
> 	* g++.dg/cpp0x/constexpr-typeid2.C: New test.
> 
> --- gcc/symtab.c.jj	2021-11-19 09:58:37.268716406 +0100
> +++ gcc/symtab.c	2021-12-09 20:41:30.085566768 +0100
> @@ -2276,6 +2276,14 @@ symtab_node::equal_address_to (symtab_no
>         return 0;
>       }
>   
> +  /* If the FE tells us at least one of the decls will never be aliased nor
> +     overlapping with other vars in some other way, return 0.  */
> +  if (VAR_P (decl)
> +      && rs1 != rs2
> +      && (lookup_attribute ("non overlapping", DECL_ATTRIBUTES (decl))
> +	  || lookup_attribute ("non overlapping", DECL_ATTRIBUTES (s2->decl))))
> +    return 0;
> +
>     /* TODO: Alias oracle basically assume that addresses of global variables
>        are different unless they are declared as alias of one to another while
>        the code folding comparisons doesn't.
> --- gcc/c-family/c-attribs.c.jj	2021-09-11 09:33:37.734334126 +0200
> +++ gcc/c-family/c-attribs.c	2021-12-09 20:44:42.593810421 +0100
> @@ -159,6 +159,7 @@ static tree handle_omp_declare_variant_a
>   static tree handle_simd_attribute (tree *, tree, tree, int, bool *);
>   static tree handle_omp_declare_target_attribute (tree *, tree, tree, int,
>   						 bool *);
> +static tree handle_non_overlapping_attribute (tree *, tree, tree, int, bool *);
>   static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
>   static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
>   						       int, bool *);
> @@ -512,6 +513,8 @@ const struct attribute_spec c_common_att
>   			      handle_omp_declare_target_attribute, NULL },
>     { "omp declare target block", 0, 0, true, false, false, false,
>   			      handle_omp_declare_target_attribute, NULL },
> +  { "non overlapping",	      0, 0, true, false, false, false,
> +			      handle_non_overlapping_attribute, NULL },
>     { "alloc_align",	      1, 1, false, true, true, false,
>   			      handle_alloc_align_attribute,
>   	                      attr_alloc_exclusions },
> @@ -3764,6 +3767,15 @@ handle_omp_declare_target_attribute (tre
>   {
>     return NULL_TREE;
>   }
> +
> +/* Handle an "non overlapping" attribute; arguments as in
> +   struct attribute_spec.handler.  */
> +
> +static tree
> +handle_non_overlapping_attribute (tree *, tree, tree, int, bool *)
> +{
> +  return NULL_TREE;
> +}
>   
>   /* Handle a "returns_twice" attribute; arguments as in
>      struct attribute_spec.handler.  */
> --- gcc/cp/rtti.c.jj	2021-12-09 15:37:05.330625874 +0100
> +++ gcc/cp/rtti.c	2021-12-09 21:02:13.090738268 +0100
> @@ -475,6 +475,15 @@ get_tinfo_decl_direct (tree type, tree n
>         DECL_IGNORED_P (d) = 1;
>         TREE_READONLY (d) = 1;
>         TREE_STATIC (d) = 1;
> +      /* Tell equal_address_to that different tinfo decls never
> +	 overlap.  */
> +      if (vec_safe_is_empty (unemitted_tinfo_decls))
> +	DECL_ATTRIBUTES (d)
> +	  = build_tree_list (get_identifier ("non overlapping"),
> +			     NULL_TREE);
> +      else
> +	DECL_ATTRIBUTES (d)
> +	  = DECL_ATTRIBUTES ((*unemitted_tinfo_decls)[0]);
>   
>         /* Mark the variable as undefined -- but remember that we can
>   	 define it later if we need to do so.  */
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C.jj	2021-12-09 20:28:47.083369305 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C	2021-12-09 20:28:47.083369305 +0100
> @@ -0,0 +1,14 @@
> +// PR c++/103600
> +// { dg-do compile { target c++11 } }
> +
> +#include <typeinfo>
> +
> +struct S { int i; };
> +namespace {
> +  struct T { int i; };
> +};
> +constexpr bool a = &typeid (int) == &typeid (int);
> +constexpr bool b = &typeid (int) == &typeid (long);
> +constexpr bool c = &typeid (double) != &typeid (int);
> +constexpr bool d = &typeid (S) != &typeid (T);
> +static_assert (a && !b && c && d, "");
> 
> 
> 	Jakub
>
  

Patch

--- gcc/symtab.c.jj	2021-11-19 09:58:37.268716406 +0100
+++ gcc/symtab.c	2021-12-09 20:41:30.085566768 +0100
@@ -2276,6 +2276,14 @@  symtab_node::equal_address_to (symtab_no
       return 0;
     }
 
+  /* If the FE tells us at least one of the decls will never be aliased nor
+     overlapping with other vars in some other way, return 0.  */
+  if (VAR_P (decl)
+      && rs1 != rs2
+      && (lookup_attribute ("non overlapping", DECL_ATTRIBUTES (decl))
+	  || lookup_attribute ("non overlapping", DECL_ATTRIBUTES (s2->decl))))
+    return 0;
+
   /* TODO: Alias oracle basically assume that addresses of global variables
      are different unless they are declared as alias of one to another while
      the code folding comparisons doesn't.
--- gcc/c-family/c-attribs.c.jj	2021-09-11 09:33:37.734334126 +0200
+++ gcc/c-family/c-attribs.c	2021-12-09 20:44:42.593810421 +0100
@@ -159,6 +159,7 @@  static tree handle_omp_declare_variant_a
 static tree handle_simd_attribute (tree *, tree, tree, int, bool *);
 static tree handle_omp_declare_target_attribute (tree *, tree, tree, int,
 						 bool *);
+static tree handle_non_overlapping_attribute (tree *, tree, tree, int, bool *);
 static tree handle_designated_init_attribute (tree *, tree, tree, int, bool *);
 static tree handle_patchable_function_entry_attribute (tree *, tree, tree,
 						       int, bool *);
@@ -512,6 +513,8 @@  const struct attribute_spec c_common_att
 			      handle_omp_declare_target_attribute, NULL },
   { "omp declare target block", 0, 0, true, false, false, false,
 			      handle_omp_declare_target_attribute, NULL },
+  { "non overlapping",	      0, 0, true, false, false, false,
+			      handle_non_overlapping_attribute, NULL },
   { "alloc_align",	      1, 1, false, true, true, false,
 			      handle_alloc_align_attribute,
 	                      attr_alloc_exclusions },
@@ -3764,6 +3767,15 @@  handle_omp_declare_target_attribute (tre
 {
   return NULL_TREE;
 }
+
+/* Handle an "non overlapping" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_non_overlapping_attribute (tree *, tree, tree, int, bool *)
+{
+  return NULL_TREE;
+}
 
 /* Handle a "returns_twice" attribute; arguments as in
    struct attribute_spec.handler.  */
--- gcc/cp/rtti.c.jj	2021-12-09 15:37:05.330625874 +0100
+++ gcc/cp/rtti.c	2021-12-09 21:02:13.090738268 +0100
@@ -475,6 +475,15 @@  get_tinfo_decl_direct (tree type, tree n
       DECL_IGNORED_P (d) = 1;
       TREE_READONLY (d) = 1;
       TREE_STATIC (d) = 1;
+      /* Tell equal_address_to that different tinfo decls never
+	 overlap.  */
+      if (vec_safe_is_empty (unemitted_tinfo_decls))
+	DECL_ATTRIBUTES (d)
+	  = build_tree_list (get_identifier ("non overlapping"),
+			     NULL_TREE);
+      else
+	DECL_ATTRIBUTES (d)
+	  = DECL_ATTRIBUTES ((*unemitted_tinfo_decls)[0]);
 
       /* Mark the variable as undefined -- but remember that we can
 	 define it later if we need to do so.  */
--- gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C.jj	2021-12-09 20:28:47.083369305 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C	2021-12-09 20:28:47.083369305 +0100
@@ -0,0 +1,14 @@ 
+// PR c++/103600
+// { dg-do compile { target c++11 } }
+
+#include <typeinfo>
+
+struct S { int i; };
+namespace {
+  struct T { int i; };
+};
+constexpr bool a = &typeid (int) == &typeid (int);
+constexpr bool b = &typeid (int) == &typeid (long);
+constexpr bool c = &typeid (double) != &typeid (int);
+constexpr bool d = &typeid (S) != &typeid (T);
+static_assert (a && !b && c && d, "");