Re: RFA (openmp): C++ PATCH to make some references TREE_CONSTANT

2016-11-15 Thread Jason Merrill 
On Tue, Nov 15, 2016 at 4:32 AM, Jakub Jelinek  wrote:
> So, is there a way to treat references the similarly?  I.e. only "fold"
> reference vars to what they refer (DECL_INITIAL) in constexpr.c evaluation,
> or gimplification where a langhook or omp_notice_variable etc. has the
> last say on when it is ok to do that or not?

Yes, we can just not fold away references in cp_fold.  Applying this instead.

Jason
commit 0f5d66d82ca526419d3d3f0ee032ea88b070b214
Author: Jason Merrill 
Date:   Mon Nov 14 14:15:57 2016 -0500

Allow references in constant-expressions.

* decl2.c (decl_maybe_constant_var_p): References qualify.
* constexpr.c (non_const_var_error): Handle references.
* init.c (constant_value_1): Always check decl_constant_var_p.
* cp-gimplify.c (cp_fold_maybe_rvalue): Don't fold references.
* error.c (dump_decl_name): Split out from dump_decl.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index e8c7702..40d1e7b 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3153,6 +3153,10 @@ non_const_var_error (tree r)
   else
gcc_unreachable ();
 }
+  else if (TREE_CODE (type) == REFERENCE_TYPE)
+inform (DECL_SOURCE_LOCATION (r),
+   "%qD was not initialized with a constant "
+   "expression", r);
   else
 {
   if (cxx_dialect >= cxx11 && !DECL_DECLARED_CONSTEXPR_P (r))
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 9b9b511..5b5c0be 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -1977,7 +1977,8 @@ cp_fold_maybe_rvalue (tree x, bool rval)
   while (true)
 {
   x = cp_fold (x);
-  if (rval && DECL_P (x))
+  if (rval && DECL_P (x)
+ && TREE_CODE (TREE_TYPE (x)) != REFERENCE_TYPE)
{
  tree v = decl_constant_value (x);
  if (v != x && v != error_mark_node)
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 4ebc7dc..257d211 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4144,6 +4144,9 @@ decl_maybe_constant_var_p (tree decl)
   if (DECL_HAS_VALUE_EXPR_P (decl))
 /* A proxy isn't constant.  */
 return false;
+  if (TREE_CODE (type) == REFERENCE_TYPE)
+/* References can be constant.  */
+return true;
   return (CP_TYPE_CONST_NON_VOLATILE_P (type)
  && INTEGRAL_OR_ENUMERATION_TYPE_P (type));
 }
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index fe1f751..7bf07c3 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1000,6 +1000,37 @@ dump_simple_decl (cxx_pretty_printer *pp, tree t, tree 
type, int flags)
 dump_type_suffix (pp, type, flags);
 }
 
+/* Print an IDENTIFIER_NODE that is the name of a declaration.  */
+
+static void
+dump_decl_name (cxx_pretty_printer *pp, tree t, int flags)
+{
+  /* These special cases are duplicated here so that other functions
+ can feed identifiers to error and get them demangled properly.  */
+  if (IDENTIFIER_TYPENAME_P (t))
+{
+  pp_cxx_ws_string (pp, "operator");
+  /* Not exactly IDENTIFIER_TYPE_VALUE.  */
+  dump_type (pp, TREE_TYPE (t), flags);
+  return;
+}
+  if (dguide_name_p (t))
+{
+  dump_decl (pp, CLASSTYPE_TI_TEMPLATE (TREE_TYPE (t)),
+TFF_PLAIN_IDENTIFIER);
+  return;
+}
+
+  const char *str = IDENTIFIER_POINTER (t);
+  if (!strncmp (str, "_ZGR", 3))
+{
+  pp_cxx_ws_string (pp, "");
+  return;
+}
+
+  pp_cxx_tree_identifier (pp, t);
+}
+
 /* Dump a human readable string for the decl T under control of FLAGS.  */
 
 static void
@@ -1155,21 +1186,8 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
   gcc_unreachable ();
   break;
 
-  /* These special cases are duplicated here so that other functions
-can feed identifiers to error and get them demangled properly.  */
 case IDENTIFIER_NODE:
-  if (IDENTIFIER_TYPENAME_P (t))
-   {
- pp_cxx_ws_string (pp, "operator");
- /* Not exactly IDENTIFIER_TYPE_VALUE.  */
- dump_type (pp, TREE_TYPE (t), flags);
- break;
-   }
-  else if (dguide_name_p (t))
-   dump_decl (pp, CLASSTYPE_TI_TEMPLATE (TREE_TYPE (t)),
-  TFF_PLAIN_IDENTIFIER);
-  else
-   pp_cxx_tree_identifier (pp, t);
+  dump_decl_name (pp, t, flags);
   break;
 
 case OVERLOAD:
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 1fad79c..b4b6cdb 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2078,10 +2078,9 @@ static tree
 constant_value_1 (tree decl, bool strict_p, bool return_aggregate_cst_ok_p)
 {
   while (TREE_CODE (decl) == CONST_DECL
-|| (strict_p
-? decl_constant_var_p (decl)
-: (VAR_P (decl)
-   && CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (decl)
+|| decl_constant_var_p (decl)
+|| (!strict_p && VAR_P (decl)
+&& CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (decl
 {
   tree init;
   /* If DECL is a static data

Re: RFA (openmp): C++ PATCH to make some references TREE_CONSTANT

2016-11-15 Thread Jakub Jelinek 
On Mon, Nov 14, 2016 at 11:53:55PM -0500, Jason Merrill wrote:
> The standard says that references that refer to a constant address can
> be used in a constant-expression, but we haven't allowed that.  This
> patch implements it, but without the parser.c hunk it broke
> libgomp.c++/target-14.C.

The parser hunk can't be right, the decl you modify TREE_CONSTANT on
is not in any way "an OpenMP variable", it is just a variable used in some
OpenMP region, modifying the flag will affect all the handling of the
variable, both outside and inside of OpenMP regions.

> Apparently the target mapping wants to use 't' in a way that doesn't
> actually refer to x?  This seems like a strange use of references.

Yes.  The privatization and mapping of variables with reference type
in OpenMP generally works by privatizing or mapping what that reference
references, and then creating a new reference that references the privatized
or mapped variable.

Looking at the *.original dump diff without your patch to with your patch
minus the parser.c hunk shows:
-  if ((*a != 8 || *c != 142) || *t != 19)
+  if ((*a != 8 || *c != 142) || *(int &)  != 19)
which might be fine outside of the OpenMP regions, from what you wrote I
understood is required in constexpr contexts, but can't be right in the
OpenMP regions - have to be deferred until omplower pass does whatever it
needs to do.  This is similar to DECL_VALUE_EXPR handling I've mentioned
yesterday, there it is also dangerous to just fold var with DECL_VALUE_EXPR
to its DECL_VALUE_EXPR until after omplower pass - the gimplifier gimplifies
such vars to their DECL_VALUE_EXPR only if it is ok (omp_notice_variable
function, e.g. together with disregard_value_expr langhook, decides on when
it is ok or not).

In target-14.C, there is mapping of reference t, so after lowering
there is going to be in the region used variable t' that refers to some int
in the target.  Your patch replaces uses of t in the region much earlier
with uses of , but that is something that isn't explicitly mapped, and
the OpenMP 4.5 rules say that such variable is firstprivatized implicitly,
so the code outside of the target region won't see any changes in the
variable.

So, is there a way to treat references the similarly?  I.e. only "fold"
reference vars to what they refer (DECL_INITIAL) in constexpr.c evaluation,
or gimplification where a langhook or omp_notice_variable etc. has the
last say on when it is ok to do that or not?

Jakub

RFA (openmp): C++ PATCH to make some references TREE_CONSTANT

2016-11-14 Thread Jason Merrill 
The standard says that references that refer to a constant address can
be used in a constant-expression, but we haven't allowed that.  This
patch implements it, but without the parser.c hunk it broke
libgomp.c++/target-14.C.

Apparently the target mapping wants to use 't' in a way that doesn't
actually refer to x?  This seems like a strange use of references.
But if this is the desired behavior, does the parser.c patch seem like
a good way to prevent the compiler from replacing 't' with 'x' within
the openmp block?

Tested x86_64-pc-linux-gnu.
commit 93b5c38cb70b9c1e52a143415da76218583242d2
Author: Jason Merrill 
Date:   Mon Nov 14 14:15:57 2016 -0500

* decl2.c (decl_maybe_constant_var_p): References qualify.
* init.c (constant_value_1): Always check decl_constant_var_p.
* parser.c (cp_parser_omp_var_list_no_open): Clear
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P on variables in OpenMP
clauses.

diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 4ebc7dc..3952600 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4127,7 +4127,7 @@ decl_constant_var_p (tree decl)
  constant until its initializer is complete in case it's used in
  its own initializer.  */
   maybe_instantiate_decl (decl);
-  return DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl);
+  return TREE_CONSTANT (decl);
 }
 
 /* Returns true if DECL could be a symbolic constant variable, depending on
@@ -4144,6 +4144,9 @@ decl_maybe_constant_var_p (tree decl)
   if (DECL_HAS_VALUE_EXPR_P (decl))
 /* A proxy isn't constant.  */
 return false;
+  if (TREE_CODE (type) == REFERENCE_TYPE)
+/* References can be constant.  */
+return true;
   return (CP_TYPE_CONST_NON_VOLATILE_P (type)
  && INTEGRAL_OR_ENUMERATION_TYPE_P (type));
 }
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index fe1f751..7bf07c3 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -1000,6 +1000,37 @@ dump_simple_decl (cxx_pretty_printer *pp, tree t, tree 
type, int flags)
 dump_type_suffix (pp, type, flags);
 }
 
+/* Print an IDENTIFIER_NODE that is the name of a declaration.  */
+
+static void
+dump_decl_name (cxx_pretty_printer *pp, tree t, int flags)
+{
+  /* These special cases are duplicated here so that other functions
+ can feed identifiers to error and get them demangled properly.  */
+  if (IDENTIFIER_TYPENAME_P (t))
+{
+  pp_cxx_ws_string (pp, "operator");
+  /* Not exactly IDENTIFIER_TYPE_VALUE.  */
+  dump_type (pp, TREE_TYPE (t), flags);
+  return;
+}
+  if (dguide_name_p (t))
+{
+  dump_decl (pp, CLASSTYPE_TI_TEMPLATE (TREE_TYPE (t)),
+TFF_PLAIN_IDENTIFIER);
+  return;
+}
+
+  const char *str = IDENTIFIER_POINTER (t);
+  if (!strncmp (str, "_ZGR", 3))
+{
+  pp_cxx_ws_string (pp, "");
+  return;
+}
+
+  pp_cxx_tree_identifier (pp, t);
+}
+
 /* Dump a human readable string for the decl T under control of FLAGS.  */
 
 static void
@@ -1155,21 +1186,8 @@ dump_decl (cxx_pretty_printer *pp, tree t, int flags)
   gcc_unreachable ();
   break;
 
-  /* These special cases are duplicated here so that other functions
-can feed identifiers to error and get them demangled properly.  */
 case IDENTIFIER_NODE:
-  if (IDENTIFIER_TYPENAME_P (t))
-   {
- pp_cxx_ws_string (pp, "operator");
- /* Not exactly IDENTIFIER_TYPE_VALUE.  */
- dump_type (pp, TREE_TYPE (t), flags);
- break;
-   }
-  else if (dguide_name_p (t))
-   dump_decl (pp, CLASSTYPE_TI_TEMPLATE (TREE_TYPE (t)),
-  TFF_PLAIN_IDENTIFIER);
-  else
-   pp_cxx_tree_identifier (pp, t);
+  dump_decl_name (pp, t, flags);
   break;
 
 case OVERLOAD:
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 1fad79c..b4b6cdb 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -2078,10 +2078,9 @@ static tree
 constant_value_1 (tree decl, bool strict_p, bool return_aggregate_cst_ok_p)
 {
   while (TREE_CODE (decl) == CONST_DECL
-|| (strict_p
-? decl_constant_var_p (decl)
-: (VAR_P (decl)
-   && CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (decl)
+|| decl_constant_var_p (decl)
+|| (!strict_p && VAR_P (decl)
+&& CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (decl
 {
   tree init;
   /* If DECL is a static data member in a template
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9360ab0..8869c46 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -30743,6 +30743,9 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum 
omp_clause_code kind,
goto skip_comma;
 
  decl = cp_parser_lookup_name_simple (parser, name, token->location);
+ if (VAR_P (decl))
+   /* Make decl_constant_var_p false for OpenMP variables.  */
+   TREE_CONSTANT (decl) = false;
  if (decl == error_mark_node)
cp_parser_name_lookup_error