public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Support &typeid(x) == &typeid(y) and typeid(x) == typeid(y) in constant evaluation [PR103600]
@ 2021-12-08 10:35 Jakub Jelinek
  2021-12-08 13:53 ` Jason Merrill
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2021-12-08 10:35 UTC (permalink / raw)
  To: Jason Merrill, Jonathan Wakely; +Cc: gcc-patches

Hi!

If the tinfo vars are emitted in the current TU, they are emitted at the end
of the compilation, and for some types they are exported from
libstdc++/libsupc++ and not emitted in the current TU at all.

The following patch allows constant folding of comparisons of typeid
addresses and makes it possible to implement P1328R1 - making type_info
operator== constexpr (Jonathan has a patch for that).

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.
and so while match.pd folds &typeid(int) == &typeid(int) because
it is equality comparison with the same operands, for different typeids
it doesn't fold it.  The first constexpr.c hunk takes care of that
during constant evaluation.  As the std::type_info class has a protected
member __name only, I think people shouldn't be comparing anything but
the starts of those vars, so we don't need to deal with offsets into
those vars etc.

The other constexpr.c hunk and rtti.c change is to allow constexpr
operator== that will in the manifestly constant evaluation just
compare the __name members for equality.  Again, the _ZTS* variables
aren't constructed yet and for some types will not be at all.
So, what this patch does is when evaluating typeid(whatever).__name
fold the INDIRECT_REF on (const std::type_info *) &_ZTIwhatever
into a CONSTRUCTOR_NO_CLEARING ctor with just __name field initialized
to &"1S" etc.

Testcase coverage includes just the &typeid comparison, for typeid
comparison it will be in the libstdc++ patch.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

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

	PR c++/103600
	* cp-tree.h (tinfo_for_constant_evaluation): Declare.
	* constexpr.c (cxx_eval_binary_expression): Evaluate
	&typeid(x) == &typeid(y) to true or false depending on whether
	the typeid is the same or not.
	(cxx_eval_indirect_ref): Fold typeid(S) to {.__name="1S"}.
	* rtti.c (tinfo_for_constant_evaluation): New function.

	* g++.dg/cpp0x/constexpr-typeid2.C: New test.

--- gcc/cp/cp-tree.h.jj	2021-12-04 11:02:03.925646583 +0100
+++ gcc/cp/cp-tree.h	2021-12-07 16:58:06.698802940 +0100
@@ -7376,6 +7376,7 @@ extern tree build_dynamic_cast			(locati
 						 tsubst_flags_t);
 extern void emit_support_tinfos			(void);
 extern bool emit_tinfo_decl			(tree);
+extern tree tinfo_for_constant_evaluation	(tree);
 extern unsigned get_pseudo_tinfo_index		(tree);
 extern tree get_pseudo_tinfo_type		(unsigned);
 extern tree build_if_nonnull			(tree, tree, tsubst_flags_t);
--- gcc/cp/constexpr.c.jj	2021-12-04 11:02:03.949646241 +0100
+++ gcc/cp/constexpr.c	2021-12-07 17:32:07.324739082 +0100
@@ -3346,6 +3346,25 @@ cxx_eval_binary_expression (const conste
 	lhs = cplus_expand_constant (lhs);
       else if (TREE_CODE (rhs) == PTRMEM_CST)
 	rhs = cplus_expand_constant (rhs);
+
+      /* Fold &typeid(x) == &typeid(y) to
+	 whether the DECL_TINFO_P vars are the same or not.  */
+      if (flag_rtti)
+	{
+	  tree op0 = lhs;
+	  tree op1 = rhs;
+	  STRIP_NOPS (op0);
+	  STRIP_NOPS (op1);
+	  if (TREE_CODE (op0) == ADDR_EXPR
+	      && VAR_P (TREE_OPERAND (op0, 0))
+	      && DECL_TINFO_P (TREE_OPERAND (op0, 0))
+	      && TREE_CODE (op1) == ADDR_EXPR
+	      && VAR_P (TREE_OPERAND (op1, 0))
+	      && DECL_TINFO_P (TREE_OPERAND (op1, 0)))
+	    r = constant_boolean_node ((TREE_OPERAND (op0, 0)
+					!= TREE_OPERAND (op1, 0))
+				       ^ is_code_eq, type);
+	}
     }
   if (code == POINTER_PLUS_EXPR && !*non_constant_p
       && integer_zerop (lhs) && !integer_zerop (rhs))
@@ -5261,6 +5280,18 @@ cxx_eval_indirect_ref (const constexpr_c
 	  STRIP_NOPS (sub);
 	  if (TREE_CODE (sub) == ADDR_EXPR)
 	    {
+	      /* For *(const std::type_info *)&_ZTIwhatever return
+		 a constructor with no clearing and __name being the
+		 tinfo name.  */
+	      if (flag_rtti
+		  && VAR_P (TREE_OPERAND (sub, 0))
+		  && DECL_TINFO_P (TREE_OPERAND (sub, 0))
+		  && !lval
+		  && TREE_TYPE (t) == const_type_info_type_node)
+		if (tree ret
+		    = tinfo_for_constant_evaluation (TREE_OPERAND (sub, 0)))
+		  return ret;
+
 	      gcc_assert (!similar_type_p
 			  (TREE_TYPE (TREE_TYPE (sub)), TREE_TYPE (t)));
 	      /* DR 1188 says we don't have to deal with this.  */
--- gcc/cp/rtti.c.jj	2021-09-15 08:55:37.569497474 +0200
+++ gcc/cp/rtti.c	2021-12-07 17:17:50.943944129 +0100
@@ -1700,4 +1700,39 @@ emit_tinfo_decl (tree decl)
     return false;
 }
 
+/* For constant evaluation purposes, return for _ZTI* DECL a CONSTRUCTOR
+   containing just __name initialized to the tinfo_name string.  */
+
+tree
+tinfo_for_constant_evaluation (tree decl)
+{
+  gcc_assert (DECL_TINFO_P (decl));
+
+  tree type = TREE_TYPE (DECL_NAME (decl));
+  if (typeinfo_in_lib_p (type))
+    ;
+  else if (involves_incomplete_p (type))
+    return NULL_TREE;
+
+  tree field
+    = next_initializable_field (TYPE_FIELDS (const_type_info_type_node));
+  if (field == NULL_TREE)
+    return NULL_TREE;
+  field = next_initializable_field (DECL_CHAIN (field));
+  if (field == NULL_TREE
+      || TREE_CODE (TREE_TYPE (field)) != POINTER_TYPE
+      || TREE_TYPE (TREE_TYPE (field)) == error_mark_node
+      || TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (field))) != char_type_node
+      || !TYPE_READONLY (TREE_TYPE (TREE_TYPE (field))))
+    return NULL_TREE;
+
+  tree str = tinfo_name (type, !TREE_PUBLIC (decl));
+  str = build_fold_addr_expr (str);
+  str = fold_convert (TREE_TYPE (field), str);
+
+  tree init = build_constructor_single (const_type_info_type_node, field, str);
+  CONSTRUCTOR_NO_CLEARING (init) = true;
+  return init;
+}
+
 #include "gt-cp-rtti.h"
--- gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C.jj	2021-12-07 19:12:02.233350194 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C	2021-12-07 19:11:43.071623082 +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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: Support &typeid(x) == &typeid(y) and typeid(x) == typeid(y) in constant evaluation [PR103600]
  2021-12-08 10:35 [PATCH] c++: Support &typeid(x) == &typeid(y) and typeid(x) == typeid(y) in constant evaluation [PR103600] Jakub Jelinek
@ 2021-12-08 13:53 ` Jason Merrill
  2021-12-08 13:56   ` Jonathan Wakely
  2021-12-09 20:15   ` [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) " Jakub Jelinek
  0 siblings, 2 replies; 7+ messages in thread
From: Jason Merrill @ 2021-12-08 13:53 UTC (permalink / raw)
  To: Jakub Jelinek, Jonathan Wakely; +Cc: gcc-patches

On 12/8/21 05:35, Jakub Jelinek wrote:
> Hi!
> 
> If the tinfo vars are emitted in the current TU, they are emitted at the end
> of the compilation, and for some types they are exported from
> libstdc++/libsupc++ and not emitted in the current TU at all.
> 
> The following patch allows constant folding of comparisons of typeid
> addresses and makes it possible to implement P1328R1 - making type_info
> operator== constexpr (Jonathan has a patch for that).
> 
> 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?

> and so while match.pd folds &typeid(int) == &typeid(int) because
> it is equality comparison with the same operands, for different typeids
> it doesn't fold it.  The first constexpr.c hunk takes care of that
> during constant evaluation.  As the std::type_info class has a protected
> member __name only, I think people shouldn't be comparing anything but
> the starts of those vars, so we don't need to deal with offsets into
> those vars etc.
> 
> The other constexpr.c hunk and rtti.c change is to allow constexpr
> operator== that will in the manifestly constant evaluation just
> compare the __name members for equality.  Again, the _ZTS* variables
> aren't constructed yet and for some types will not be at all.
> So, what this patch does is when evaluating typeid(whatever).__name
> fold the INDIRECT_REF on (const std::type_info *) &_ZTIwhatever
> into a CONSTRUCTOR_NO_CLEARING ctor with just __name field initialized
> to &"1S" etc.

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

> Testcase coverage includes just the &typeid comparison, for typeid
> comparison it will be in the libstdc++ patch.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2021-12-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/103600
> 	* cp-tree.h (tinfo_for_constant_evaluation): Declare.
> 	* constexpr.c (cxx_eval_binary_expression): Evaluate
> 	&typeid(x) == &typeid(y) to true or false depending on whether
> 	the typeid is the same or not.
> 	(cxx_eval_indirect_ref): Fold typeid(S) to {.__name="1S"}.
> 	* rtti.c (tinfo_for_constant_evaluation): New function.
> 
> 	* g++.dg/cpp0x/constexpr-typeid2.C: New test.
> 
> --- gcc/cp/cp-tree.h.jj	2021-12-04 11:02:03.925646583 +0100
> +++ gcc/cp/cp-tree.h	2021-12-07 16:58:06.698802940 +0100
> @@ -7376,6 +7376,7 @@ extern tree build_dynamic_cast			(locati
>   						 tsubst_flags_t);
>   extern void emit_support_tinfos			(void);
>   extern bool emit_tinfo_decl			(tree);
> +extern tree tinfo_for_constant_evaluation	(tree);
>   extern unsigned get_pseudo_tinfo_index		(tree);
>   extern tree get_pseudo_tinfo_type		(unsigned);
>   extern tree build_if_nonnull			(tree, tree, tsubst_flags_t);
> --- gcc/cp/constexpr.c.jj	2021-12-04 11:02:03.949646241 +0100
> +++ gcc/cp/constexpr.c	2021-12-07 17:32:07.324739082 +0100
> @@ -3346,6 +3346,25 @@ cxx_eval_binary_expression (const conste
>   	lhs = cplus_expand_constant (lhs);
>         else if (TREE_CODE (rhs) == PTRMEM_CST)
>   	rhs = cplus_expand_constant (rhs);
> +
> +      /* Fold &typeid(x) == &typeid(y) to
> +	 whether the DECL_TINFO_P vars are the same or not.  */
> +      if (flag_rtti)
> +	{
> +	  tree op0 = lhs;
> +	  tree op1 = rhs;
> +	  STRIP_NOPS (op0);
> +	  STRIP_NOPS (op1);
> +	  if (TREE_CODE (op0) == ADDR_EXPR
> +	      && VAR_P (TREE_OPERAND (op0, 0))
> +	      && DECL_TINFO_P (TREE_OPERAND (op0, 0))
> +	      && TREE_CODE (op1) == ADDR_EXPR
> +	      && VAR_P (TREE_OPERAND (op1, 0))
> +	      && DECL_TINFO_P (TREE_OPERAND (op1, 0)))
> +	    r = constant_boolean_node ((TREE_OPERAND (op0, 0)
> +					!= TREE_OPERAND (op1, 0))
> +				       ^ is_code_eq, type);
> +	}
>       }
>     if (code == POINTER_PLUS_EXPR && !*non_constant_p
>         && integer_zerop (lhs) && !integer_zerop (rhs))
> @@ -5261,6 +5280,18 @@ cxx_eval_indirect_ref (const constexpr_c
>   	  STRIP_NOPS (sub);
>   	  if (TREE_CODE (sub) == ADDR_EXPR)
>   	    {
> +	      /* For *(const std::type_info *)&_ZTIwhatever return
> +		 a constructor with no clearing and __name being the
> +		 tinfo name.  */
> +	      if (flag_rtti
> +		  && VAR_P (TREE_OPERAND (sub, 0))
> +		  && DECL_TINFO_P (TREE_OPERAND (sub, 0))
> +		  && !lval
> +		  && TREE_TYPE (t) == const_type_info_type_node)
> +		if (tree ret
> +		    = tinfo_for_constant_evaluation (TREE_OPERAND (sub, 0)))
> +		  return ret;
> +
>   	      gcc_assert (!similar_type_p
>   			  (TREE_TYPE (TREE_TYPE (sub)), TREE_TYPE (t)));
>   	      /* DR 1188 says we don't have to deal with this.  */
> --- gcc/cp/rtti.c.jj	2021-09-15 08:55:37.569497474 +0200
> +++ gcc/cp/rtti.c	2021-12-07 17:17:50.943944129 +0100
> @@ -1700,4 +1700,39 @@ emit_tinfo_decl (tree decl)
>       return false;
>   }
>   
> +/* For constant evaluation purposes, return for _ZTI* DECL a CONSTRUCTOR
> +   containing just __name initialized to the tinfo_name string.  */
> +
> +tree
> +tinfo_for_constant_evaluation (tree decl)
> +{
> +  gcc_assert (DECL_TINFO_P (decl));
> +
> +  tree type = TREE_TYPE (DECL_NAME (decl));
> +  if (typeinfo_in_lib_p (type))
> +    ;
> +  else if (involves_incomplete_p (type))
> +    return NULL_TREE;
> +
> +  tree field
> +    = next_initializable_field (TYPE_FIELDS (const_type_info_type_node));
> +  if (field == NULL_TREE)
> +    return NULL_TREE;
> +  field = next_initializable_field (DECL_CHAIN (field));
> +  if (field == NULL_TREE
> +      || TREE_CODE (TREE_TYPE (field)) != POINTER_TYPE
> +      || TREE_TYPE (TREE_TYPE (field)) == error_mark_node
> +      || TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (field))) != char_type_node
> +      || !TYPE_READONLY (TREE_TYPE (TREE_TYPE (field))))
> +    return NULL_TREE;
> +
> +  tree str = tinfo_name (type, !TREE_PUBLIC (decl));
> +  str = build_fold_addr_expr (str);
> +  str = fold_convert (TREE_TYPE (field), str);
> +
> +  tree init = build_constructor_single (const_type_info_type_node, field, str);
> +  CONSTRUCTOR_NO_CLEARING (init) = true;
> +  return init;
> +}
> +
>   #include "gt-cp-rtti.h"
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C.jj	2021-12-07 19:12:02.233350194 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-typeid2.C	2021-12-07 19:11:43.071623082 +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
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: Support &typeid(x) == &typeid(y) and typeid(x) == typeid(y) in constant evaluation [PR103600]
  2021-12-08 13:53 ` Jason Merrill
@ 2021-12-08 13:56   ` Jonathan Wakely
  2021-12-09 20:15   ` [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) " Jakub Jelinek
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2021-12-08 13:56 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Jakub Jelinek, gcc Patches

On Wed, 8 Dec 2021 at 13:53, Jason Merrill wrote:
> During constant evaluation, the operator== could compare the type_info
> address instead of the __name address, reducing this to the previous
> problem.

That makes sense to me. We might still want the libstdc++ changes in
case other compilers choose to do this differently. I'll get in touch
with some Clang and EDG people about that.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) in constant evaluation [PR103600]
  2021-12-08 13:53 ` Jason Merrill
  2021-12-08 13:56   ` Jonathan Wakely
@ 2021-12-09 20:15   ` Jakub Jelinek
  2021-12-09 20:41     ` Jan Hubicka
  2021-12-09 23:09     ` Jason Merrill
  1 sibling, 2 replies; 7+ messages in thread
From: Jakub Jelinek @ 2021-12-09 20:15 UTC (permalink / raw)
  To: Jason Merrill, Jan Hubicka; +Cc: Jonathan Wakely, gcc-patches

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.

--- 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) in constant evaluation [PR103600]
  2021-12-09 20:15   ` [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) " Jakub Jelinek
@ 2021-12-09 20:41     ` Jan Hubicka
  2021-12-09 23:09     ` Jason Merrill
  1 sibling, 0 replies; 7+ messages in thread
From: Jan Hubicka @ 2021-12-09 20:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, Jonathan Wakely, gcc-patches

> 
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) in constant evaluation [PR103600]
  2021-12-09 20:15   ` [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) " Jakub Jelinek
  2021-12-09 20:41     ` Jan Hubicka
@ 2021-12-09 23:09     ` Jason Merrill
  2021-12-10 10:41       ` [PATCH] symtab: Fold &a == &b to 0 if folding_initializer [PR94716] Jakub Jelinek
  1 sibling, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2021-12-09 23:09 UTC (permalink / raw)
  To: Jakub Jelinek, Jan Hubicka; +Cc: Jonathan Wakely, gcc-patches

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
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] symtab: Fold &a == &b to 0 if folding_initializer [PR94716]
  2021-12-09 23:09     ` Jason Merrill
@ 2021-12-10 10:41       ` Jakub Jelinek
  0 siblings, 0 replies; 7+ messages in thread
From: Jakub Jelinek @ 2021-12-10 10:41 UTC (permalink / raw)
  To: Jason Merrill, Jan Hubicka; +Cc: Jonathan Wakely, gcc-patches

Hi!

On Thu, Dec 09, 2021 at 06:09:12PM -0500, Jason Merrill wrote:
> 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.

I agree.  This is an incremental patch to do that (so far untested),
Honza, what do you think about it?

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

	PR c++/94716
gcc/
	* symtab.c: Include fold-const.h.
	(symtab_node::equal_address_to): If folding_initializer is true,
	handle it like memory_accessed.  Simplify.
gcc/testsuite/
	* gcc.dg/init-compare-1.c: New test.
	* g++.dg/cpp0x/constexpr-compare1.C: New test.
	* g++.dg/cpp1y/constexpr-94716.C: New test.
	* g++.dg/cpp1z/constexpr-compare1.C: New test.

--- gcc/symtab.c.jj	2021-12-09 20:41:30.085566768 +0100
+++ gcc/symtab.c	2021-12-10 11:14:56.072577457 +0100
@@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.
 #include "stringpool.h"
 #include "attribs.h"
 #include "builtins.h"
+#include "fold-const.h"
 
 static const char *ipa_ref_use_name[] = {"read","write","addr","alias"};
 
@@ -2276,10 +2277,12 @@ symtab_node::equal_address_to (symtab_no
       return 0;
     }
 
+  if (rs1 == rs2)
+    return -1;
+
   /* 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;
@@ -2288,9 +2291,14 @@ symtab_node::equal_address_to (symtab_no
      are different unless they are declared as alias of one to another while
      the code folding comparisons doesn't.
      We probably should be consistent and use this fact here, too, but for
-     the moment return false only when we are called from the alias oracle.  */
+     the moment return false only when we are called from the alias oracle.
+     Return 0 in C constant initializers and C++ manifestly constant
+     expressions, the likelyhood that different vars will be aliases is
+     small and returning -1 lets us reject too many initializers.  */
+  if (memory_accessed || folding_initializer)
+    return 0;
 
-  return memory_accessed && rs1 != rs2 ? 0 : -1;
+  return -1;
 }
 
 /* Worker for call_for_symbol_and_aliases.  */
--- gcc/testsuite/gcc.dg/init-compare-1.c.jj	2021-12-10 11:31:45.839084036 +0100
+++ gcc/testsuite/gcc.dg/init-compare-1.c	2021-12-10 11:32:08.551757661 +0100
@@ -0,0 +1,5 @@
+/* { dg-do compile } */
+
+extern int a, b;
+int c = &a == &a;
+int d = &a != &b;
--- gcc/testsuite/g++.dg/cpp0x/constexpr-compare1.C.jj	2021-12-10 11:25:58.579074051 +0100
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-compare1.C	2021-12-10 11:31:38.100195242 +0100
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+extern int a, b;
+static_assert (&a == &a, "");
+static_assert (&a != &b, "");
+constexpr bool c = &a == &a;
+constexpr bool d = &a != &b;
--- gcc/testsuite/g++.dg/cpp1y/constexpr-94716.C.jj	2021-12-10 11:24:20.028490189 +0100
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-94716.C	2021-12-10 11:35:52.782538861 +0100
@@ -0,0 +1,8 @@
+// PR c++/94716
+// { dg-do compile { target c++14 } }
+
+template <int> char v = 0;
+static_assert (&v<2> == &v<2>, "");
+static_assert (&v<0> != &v<1>, "");
+constexpr bool a = &v<2> == &v<2>;
+constexpr bool b = &v<0> != &v<1>;
--- gcc/testsuite/g++.dg/cpp1z/constexpr-compare1.C.jj	2021-12-10 11:28:31.642874577 +0100
+++ gcc/testsuite/g++.dg/cpp1z/constexpr-compare1.C	2021-12-10 11:30:59.114755454 +0100
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++17 } }
+
+inline int a = 0;
+inline int b = 0;
+static_assert (&a == &a);
+static_assert (&a != &b);
+constexpr bool c = &a == &a;
+constexpr bool d = &a != &b;


	Jakub


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-12-10 10:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 10:35 [PATCH] c++: Support &typeid(x) == &typeid(y) and typeid(x) == typeid(y) in constant evaluation [PR103600] Jakub Jelinek
2021-12-08 13:53 ` Jason Merrill
2021-12-08 13:56   ` Jonathan Wakely
2021-12-09 20:15   ` [PATCH] c++, symtab: Support &typeid(x) == &typeid(y) " Jakub Jelinek
2021-12-09 20:41     ` Jan Hubicka
2021-12-09 23:09     ` Jason Merrill
2021-12-10 10:41       ` [PATCH] symtab: Fold &a == &b to 0 if folding_initializer [PR94716] Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).