On Wed, Oct 09, 2013 at 10:47:22AM -0400, Jason Merrill wrote:
> On 10/07/2013 07:02 AM, Jakub Jelinek wrote:
> >duplicates anywhere, but during error diagnostics. Without those two decl.c
> >hunks (either of them), pushdecl will sometimes return a different decl from
> >the original or error_mark_node, and the original fndecl passed to it has
> >ggc_free called on it, thus any further use of it ICEs or may ICE.
>
> Right.
>
> >Perhaps if pushdecl returns error_mark_node,
> >then I'd should expect that the error has been reported already and if
> >it returns some other FUNCTION_DECL, then I should report it myself,
>
> Makes sense.
>
> >but a problem with that is that there are multiple locations that call
> >pushdecl (two in parser.c, one in pt.c) and more importantly, that for
> >the diagnostics the new fndecl is ggc_freed and thus I can't use it
> >for the diagnostics anymore.
>
> True, though probably input_location is enough.
It turned much easier to just diagnose and return error_mark_node
for UDRs always in duplicate_decls by just moving the if a few lines
earlier, then no changes in decls_match are needed.
> >>Normal C++ lookup behavior is to check for ambiguity, so I think
> >>that's the best bet for what the eventual defined semantics will be.
> >
> >No response from omp-lang yet, so I'm not changing this yet.
>
> Please do change it. The current behavior is just wrong, and we
> should set a good example for others to follow. It's ok to fix this
> in a follow-up patch.
Ok, see the attached patch.
>
> >Unfortunately it didn't work, again on the udr-3.C testcase.
> >mark_used was already called during instantiation of the decl, DECL_ODR_USED
> >got set on it, but it was actually deferred, then when mark_used is called
> >again on it, it is ignored. I'd need to clear DECL_ODR_USED explicitly
> >and call mark_used, perhaps that would work.
>
> If deferring it is a problem you can add UDRs to the group of things
> which are always instantiated immediately in mark_used:
Ok, forced it there. Just to be sure moved the DECL_LANG_SPECIFIC check
first, because DECL_OMP_DECLARE_REDUCTION_P is in DECL_LANG_SPECIFIC.
Perhaps DECL_TEMPLATE_INFO check could also be less expensive to be done
before calling decl_maybe_constant_var_p or undeduced_auto_decl ?
> >+ error_at (loc, "predeclared arithmetic type in %qT"
> >+ error_at (loc, "reference type in %qT"
>
> "%qT in"
Fixed.
Ok?
2013-10-09 Jakub Jelinek <ja...@redhat.com>
* decl.c (decls_match): Revert DECL_OMP_DECLARE_REDUCTION_P
special cases.
(duplicate_decls): Move DECL_OMP_DECLARE_REDUCTION_P case
earlier.
* parser.c (cp_parser_omp_declare_reduction): Fix spelling
of some error messages. Set DECL_CONTEXT (fndecl) to
global_namespace first and set DECL_LOCAL_FUNCTION_P (fndecl).
* decl2.c (mark_used): Force immediate instantiation of
DECL_OMP_DECLARE_REDUCTION_P decls.
* semantics.c (omp_reduction_lookup): Add baselinkp and
ambiguousp arguments, diagnose ambiguities, perform access
check only if non-ambiguous.
(finish_omp_reduction_clause): Adjust omp_reduction_lookup
caller, if it returned error_mark_node, just return true,
use mark_used instead of instantiate_decl.
gcc/testsuite/
* g++.dg/gomp/udr-6.C: New test.
libgomp/
* testsuite/libgomp.c++/udr-6.C: Remove UDR + on type F.
--- gcc/cp/decl.c.jj 2013-10-07 14:06:58.000000000 +0200
+++ gcc/cp/decl.c 2013-10-09 17:41:58.264948392 +0200
@@ -978,15 +978,12 @@ decls_match (tree newdecl, tree olddecl)
tree t2 = (DECL_USE_TEMPLATE (olddecl)
? DECL_TI_TEMPLATE (olddecl)
: NULL_TREE);
- if (t1 != t2 && !DECL_OMP_DECLARE_REDUCTION_P (newdecl))
+ if (t1 != t2)
return 0;
if (CP_DECL_CONTEXT (newdecl) != CP_DECL_CONTEXT (olddecl)
&& ! (DECL_EXTERN_C_P (newdecl)
- && DECL_EXTERN_C_P (olddecl))
- && ! (DECL_OMP_DECLARE_REDUCTION_P (newdecl)
- && DECL_CONTEXT (newdecl) == NULL_TREE
- && DECL_CONTEXT (olddecl) == current_function_decl))
+ && DECL_EXTERN_C_P (olddecl)))
return 0;
/* A new declaration doesn't match a built-in one unless it
@@ -1344,6 +1341,15 @@ duplicate_decls (tree newdecl, tree oldd
}
return NULL_TREE;
}
+ else if (DECL_OMP_DECLARE_REDUCTION_P (olddecl))
+ {
+ gcc_assert (DECL_OMP_DECLARE_REDUCTION_P (newdecl));
+ error_at (DECL_SOURCE_LOCATION (newdecl),
+ "redeclaration of %<pragma omp declare reduction%>");
+ error_at (DECL_SOURCE_LOCATION (olddecl),
+ "previous %<pragma omp declare reduction%> declaration");
+ return error_mark_node;
+ }
else if (!types_match)
{
/* Avoid warnings redeclaring built-ins which have not been
@@ -1422,15 +1428,6 @@ duplicate_decls (tree newdecl, tree oldd
type = cp_build_type_attribute_variant (type, attribs);
TREE_TYPE (newdecl) = TREE_TYPE (olddecl) = type;
}
- else if (DECL_OMP_DECLARE_REDUCTION_P (olddecl))
- {
- gcc_assert (DECL_OMP_DECLARE_REDUCTION_P (newdecl));
- error_at (DECL_SOURCE_LOCATION (newdecl),
- "redeclaration of %<pragma omp declare reduction%>");
- error_at (DECL_SOURCE_LOCATION (olddecl),
- "previous %<pragma omp declare reduction%> declaration");
- return error_mark_node;
- }
/* If a function is explicitly declared "throw ()", propagate that to
the corresponding builtin. */
--- gcc/cp/parser.c.jj 2013-10-07 23:29:44.000000000 +0200
+++ gcc/cp/parser.c 2013-10-09 19:23:01.656235024 +0200
@@ -30131,7 +30131,7 @@ cp_parser_omp_declare_reduction (cp_pars
"min") == 0
|| strcmp (IDENTIFIER_POINTER (orig_reduc_id),
"max") == 0))))
- error_at (loc, "predeclared arithmetic type in %qT"
+ error_at (loc, "predeclared arithmetic type %qT in "
"%<#pragma omp declare reduction%>", type);
else if (TREE_CODE (type) == FUNCTION_TYPE
|| TREE_CODE (type) == METHOD_TYPE
@@ -30139,7 +30139,7 @@ cp_parser_omp_declare_reduction (cp_pars
error_at (loc, "function or array type %qT in "
"%<#pragma omp declare reduction%>", type);
else if (TREE_CODE (type) == REFERENCE_TYPE)
- error_at (loc, "reference type in %qT"
+ error_at (loc, "reference type %qT in "
"%<#pragma omp declare reduction%>", type);
else if (TYPE_QUALS_NO_ADDR_SPACE (type))
error_at (loc, "const, volatile or __restrict qualified type %qT in "
@@ -30197,6 +30197,8 @@ cp_parser_omp_declare_reduction (cp_pars
if (current_function_decl)
{
block_scope = true;
+ DECL_CONTEXT (fndecl) = global_namespace;
+ DECL_LOCAL_FUNCTION_P (fndecl) = 1;
if (!processing_template_decl)
pushdecl (fndecl);
}
--- gcc/cp/decl2.c.jj 2013-10-07 14:07:00.000000000 +0200
+++ gcc/cp/decl2.c 2013-10-09 18:14:25.958073742 +0200
@@ -4686,12 +4686,15 @@ mark_used (tree decl, tsubst_flags_t com
or a constexpr function, we need it right now because a reference to
such a data member or a call to such function is not value-dependent.
For a function that uses auto in the return type, we need to instantiate
- it to find out its type. */
- if ((decl_maybe_constant_var_p (decl)
- || (TREE_CODE (decl) == FUNCTION_DECL
- && DECL_DECLARED_CONSTEXPR_P (decl))
- || undeduced_auto_decl (decl))
- && DECL_LANG_SPECIFIC (decl)
+ it to find out its type. For OpenMP user defined reductions, we need
+ them instantiated for reduction clauses which inline them by hand
+ directly. */
+ if (DECL_LANG_SPECIFIC (decl)
+ && (decl_maybe_constant_var_p (decl)
+ || (TREE_CODE (decl) == FUNCTION_DECL
+ && (DECL_DECLARED_CONSTEXPR_P (decl)
+ || DECL_OMP_DECLARE_REDUCTION_P (decl)))
+ || undeduced_auto_decl (decl))
&& DECL_TEMPLATE_INFO (decl)
&& !uses_template_parms (DECL_TI_ARGS (decl)))
{
--- gcc/cp/semantics.c.jj 2013-10-08 08:44:15.000000000 +0200
+++ gcc/cp/semantics.c 2013-10-09 19:10:23.469074632 +0200
@@ -4601,9 +4601,11 @@ omp_reduction_id (enum tree_code reducti
FUNCTION_DECL or NULL_TREE if not found. */
static tree
-omp_reduction_lookup (location_t loc, tree id, tree type)
+omp_reduction_lookup (location_t loc, tree id, tree type, tree *baselinkp,
+ vec<tree> *ambiguousp)
{
tree orig_id = id;
+ tree baselink = NULL_TREE;
if (identifier_p (id))
{
cp_id_kind idk;
@@ -4643,20 +4645,61 @@ omp_reduction_lookup (location_t loc, tr
break;
}
}
+ if (id && BASELINK_P (fns))
+ {
+ if (baselinkp)
+ *baselinkp = fns;
+ else
+ baselink = fns;
+ }
if (id == NULL_TREE && CLASS_TYPE_P (type) && TYPE_BINFO (type))
{
- tree binfo = TYPE_BINFO (type), base_binfo;
+ vec<tree> ambiguous = vNULL;
+ tree binfo = TYPE_BINFO (type), base_binfo, ret = NULL_TREE;
unsigned int ix;
+ if (ambiguousp == NULL)
+ ambiguousp = &ambiguous;
for (ix = 0; BINFO_BASE_ITERATE (binfo, ix, base_binfo); ix++)
{
- id = omp_reduction_lookup (loc, orig_id, BINFO_TYPE (base_binfo));
- if (id != NULL_TREE)
- return id;
+ id = omp_reduction_lookup (loc, orig_id, BINFO_TYPE (base_binfo),
+ baselinkp ? baselinkp : &baselink,
+ ambiguousp);
+ if (id == NULL_TREE)
+ continue;
+ if (!ambiguousp->is_empty ())
+ ambiguousp->safe_push (id);
+ else if (ret != NULL_TREE)
+ {
+ ambiguousp->safe_push (ret);
+ ambiguousp->safe_push (id);
+ ret = NULL_TREE;
+ }
+ else
+ ret = id;
}
+ if (ambiguousp != &ambiguous)
+ return ret;
+ if (!ambiguous.is_empty ())
+ {
+ const char *str = _("candidates are:");
+ unsigned int idx;
+ tree udr;
+ error_at (loc, "user defined reduction lookup is ambiguous");
+ FOR_EACH_VEC_ELT (ambiguous, idx, udr)
+ {
+ inform (DECL_SOURCE_LOCATION (udr), "%s %#D", str, udr);
+ if (idx == 0)
+ str = get_spaces (str);
+ }
+ ambiguous.release ();
+ ret = error_mark_node;
+ baselink = NULL_TREE;
+ }
+ id = ret;
}
- if (id && BASELINK_P (fns))
- perform_or_defer_access_check (BASELINK_BINFO (fns), id, id,
- tf_warning_or_error);
+ if (id && baselink)
+ perform_or_defer_access_check (BASELINK_BINFO (baselink),
+ id, id, tf_warning_or_error);
return id;
}
@@ -4949,12 +4992,13 @@ finish_omp_reduction_clause (tree c, boo
if (id == NULL_TREE)
id = omp_reduction_id (OMP_CLAUSE_REDUCTION_CODE (c),
NULL_TREE, NULL_TREE);
- id = omp_reduction_lookup (OMP_CLAUSE_LOCATION (c), id, type);
+ id = omp_reduction_lookup (OMP_CLAUSE_LOCATION (c), id, type, NULL, NULL);
if (id)
{
+ if (id == error_mark_node)
+ return true;
id = OVL_CURRENT (id);
- if (DECL_TEMPLATE_INFO (id))
- id = instantiate_decl (id, /*defer_ok*/0, true);
+ mark_used (id);
tree body = DECL_SAVED_TREE (id);
if (TREE_CODE (body) == STATEMENT_LIST)
{
--- gcc/testsuite/g++.dg/gomp/udr-6.C.jj 2013-10-09 19:20:45.284925551
+0200
+++ gcc/testsuite/g++.dg/gomp/udr-6.C 2013-10-09 19:20:39.082957547 +0200
@@ -0,0 +1,59 @@
+// { dg-do compile }
+
+struct A { int a; A () : a (0) {} };
+struct B { int b; B () : b (0) {} };
+struct C : public A, B { int c; C () : c (0) {} };
+struct D { int d; D () : d (0) {} };
+struct E { int e; E () : e (0) {} };
+struct F : public D, E { int f; F () : f (0) {} };
+struct G : public C, F { int g; G () : g (0) {} };
+
+#pragma omp declare reduction (+: A : omp_out.a += omp_in.a) // { dg-message
"operator" }
+#pragma omp declare reduction (+: B : omp_out.b += omp_in.b) // { dg-message
"operator" }
+#pragma omp declare reduction (+: D : omp_out.d += omp_in.d)
+#pragma omp declare reduction (+: E : omp_out.e += omp_in.e)
+#pragma omp declare reduction (+: F : omp_out.f += omp_in.f) // { dg-message
"operator" }
+
+void
+f1 ()
+{
+ G g;
+ #pragma omp parallel reduction (+: g) // { dg-error "user defined
reduction lookup is ambiguous" }
+ {
+ g.g++;
+ }
+}
+
+#pragma omp declare reduction (*: A : omp_out.a += omp_in.a)
+#pragma omp declare reduction (*: B : omp_out.b += omp_in.b)
+#pragma omp declare reduction (*: D : omp_out.d += omp_in.d)
+#pragma omp declare reduction (*: E : omp_out.e += omp_in.e)
+#pragma omp declare reduction (*: F : omp_out.f += omp_in.f)
+#pragma omp declare reduction (*: G : omp_out.g += omp_in.g)
+
+void
+f2 ()
+{
+ G g;
+ #pragma omp parallel reduction (*: g)
+ {
+ g.g++;
+ }
+}
+
+#pragma omp declare reduction (|: A : omp_out.a += omp_in.a)
+#pragma omp declare reduction (|: B : omp_out.b += omp_in.b)
+#pragma omp declare reduction (|: C : omp_out.c += omp_in.c) // { dg-message
"operator" }
+#pragma omp declare reduction (|: D : omp_out.d += omp_in.d)
+#pragma omp declare reduction (|: E : omp_out.e += omp_in.e)
+#pragma omp declare reduction (|: F : omp_out.f += omp_in.f) // { dg-message
"operator" }
+
+void
+f3 ()
+{
+ G g;
+ #pragma omp parallel reduction (|: g) // { dg-error "user defined
reduction lookup is ambiguous" }
+ {
+ g.g++;
+ }
+}
--- libgomp/testsuite/libgomp.c++/udr-6.C.jj 2013-09-18 12:43:23.000000000
+0200
+++ libgomp/testsuite/libgomp.c++/udr-6.C 2013-10-09 19:25:04.450612965
+0200
@@ -11,8 +11,6 @@ struct F : C, D {};
struct G : E, F {};
void foo (B &);
void foo (F &);
-#pragma omp declare reduction (+:F:omp_out.c += omp_in.c) \
- initializer(foo (omp_priv))
#pragma omp declare reduction (+:B:omp_out.b += omp_in.b) \
initializer(foo (omp_priv))
Jakub
