C++ PATCH: PR 26559
Mark Mitchell
mark@codesourcery.com
Thu Jun 15 03:47:00 GMT 2006
This patch fixes PR c++/26559. There, we ICE'd on a use of
__builtin_constant_p in a template. A call to __builtin_constant_p is
valid in an integral constant expression -- but that doesn't mean we
can actually fold the argument to __builtin_constant_p before
instantiating the template. The bug here was that we did not consider
the call to be value-dependent.
I was very surprised to find that fixing that caused one of the OpenMP
tests to fail. The problem there turned out to be that
finish_omp_atomic in the C++ front end was not following the standard
pattern for dealing with templates. In particular, in a template, if
the arguments are not type-dependent:
1) Call non_dependent_expr.
2) Do the normal semantic processing.
3) Build a template representation of the expression, using the
original operands.
Then, during template instantiation, substitute the arguments and call
back to the same semantic routine.
Reimplementing finish_omp_atomic to follow that pattern fixed the
problem -- and an XFAIL in one of the OpenMP test cases, to boot.
Tested on x86_64-unknown-linux-gnu, applied on the mainline. I'll
backport the core part of the change to the 4.1 branch.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
2006-06-14 Mark Mitchell <mark@codesourcery.com>
PR c++/26559
* c-common.h (c_finish_omp_atomic): Adjust declaration.
* c-omp.c (c_finish_omp_atomic): Return the expression to perform,
rather than calling add_stmt on it.
* c-parser.c (c_parser_omp_atomic): Adjust accordingly.
2006-06-14 Mark Mitchell <mark@codesourcery.com>
PR c++/26559
* pt.c (tsubst_expr): Use finish_omp_atomic.
(value_dependent_expression_p): All CALL_EXPRs are dependent.
* semantics.c (finish_omp_atomic): Rework to use standard
paradigms for handling non-dependent expressions.
2006-06-14 Mark Mitchell <mark@codesourcery.com>
PR c++/26559
* g++.dg/template/builtin1.C: New test.
* g++.dg/gomp/tpl-atomic-2.C: Remove XFAIL.
Index: gcc/c-omp.c
===================================================================
--- gcc/c-omp.c (revision 114635)
+++ gcc/c-omp.c (working copy)
@@ -82,15 +82,18 @@ c_finish_omp_barrier (void)
/* Complete a #pragma omp atomic construct. The expression to be
- implemented atomically is LHS code= RHS. */
+ implemented atomically is LHS code= RHS. The value returned is
+ either error_mark_node (if the construct was erroneous) or an
+ OMP_ATOMIC node which should be added to the current statement tree
+ with add_stmt. */
-void
+tree
c_finish_omp_atomic (enum tree_code code, tree lhs, tree rhs)
{
tree x, type, addr;
if (lhs == error_mark_node || rhs == error_mark_node)
- return;
+ return error_mark_node;
/* ??? According to one reading of the OpenMP spec, complex type are
supported, but there are no atomic stores for any architecture.
@@ -102,7 +105,7 @@ c_finish_omp_atomic (enum tree_code code
&& !SCALAR_FLOAT_TYPE_P (type))
{
error ("invalid expression type for %<#pragma omp atomic%>");
- return;
+ return error_mark_node;
}
/* ??? Validate that rhs does not overlap lhs. */
@@ -111,7 +114,7 @@ c_finish_omp_atomic (enum tree_code code
via indirection. */
addr = build_unary_op (ADDR_EXPR, lhs, 0);
if (addr == error_mark_node)
- return;
+ return error_mark_node;
addr = save_expr (addr);
lhs = build_indirect_ref (addr, NULL);
@@ -120,12 +123,12 @@ c_finish_omp_atomic (enum tree_code code
to do this, and then take it apart again. */
x = build_modify_expr (lhs, code, rhs);
if (x == error_mark_node)
- return;
+ return error_mark_node;
gcc_assert (TREE_CODE (x) == MODIFY_EXPR);
rhs = TREE_OPERAND (x, 1);
/* Punt the actual generation of atomic operations to common code. */
- add_stmt (build2 (OMP_ATOMIC, void_type_node, addr, rhs));
+ return build2 (OMP_ATOMIC, void_type_node, addr, rhs);
}
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h (revision 114635)
+++ gcc/c-common.h (working copy)
@@ -948,7 +948,7 @@ extern tree c_finish_omp_master (tree);
extern tree c_finish_omp_critical (tree, tree);
extern tree c_finish_omp_ordered (tree);
extern void c_finish_omp_barrier (void);
-extern void c_finish_omp_atomic (enum tree_code, tree, tree);
+extern tree c_finish_omp_atomic (enum tree_code, tree, tree);
extern void c_finish_omp_flush (void);
extern tree c_finish_omp_for (location_t, tree, tree, tree, tree, tree, tree);
extern void c_split_parallel_clauses (tree, tree *, tree *);
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c (revision 114635)
+++ gcc/c-parser.c (working copy)
@@ -7214,6 +7214,7 @@ static void
c_parser_omp_atomic (c_parser *parser)
{
tree lhs, rhs;
+ tree stmt;
enum tree_code code;
c_parser_skip_to_pragma_eol (parser);
@@ -7280,7 +7281,9 @@ c_parser_omp_atomic (c_parser *parser)
rhs = c_parser_expression (parser).value;
break;
}
- c_finish_omp_atomic (code, lhs, rhs);
+ stmt = c_finish_omp_atomic (code, lhs, rhs);
+ if (stmt != error_mark_node)
+ add_stmt (stmt);
c_parser_skip_until_found (parser, CPP_SEMICOLON, "expected %<;%>");
}
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c (revision 114635)
+++ gcc/cp/pt.c (working copy)
@@ -8546,10 +8546,7 @@ tsubst_expr (tree t, tree args, tsubst_f
tree op0, op1;
op0 = tsubst_expr (TREE_OPERAND (t, 0), args, complain, in_decl);
op1 = tsubst_expr (TREE_OPERAND (t, 1), args, complain, in_decl);
- if (OMP_ATOMIC_DEPENDENT_P (t))
- c_finish_omp_atomic (OMP_ATOMIC_CODE (t), op0, op1);
- else
- add_stmt (build2 (OMP_ATOMIC, void_type_node, op0, op1));
+ finish_omp_atomic (OMP_ATOMIC_CODE (t), op0, op1);
}
break;
@@ -12473,7 +12470,8 @@ dependent_scope_ref_p (tree expression,
}
/* Returns TRUE if the EXPRESSION is value-dependent, in the sense of
- [temp.dep.constexpr] */
+ [temp.dep.constexpr]. EXPRESSION is already known to be a constant
+ expression. */
bool
value_dependent_expression_p (tree expression)
@@ -12564,32 +12562,10 @@ value_dependent_expression_p (tree expre
|| value_dependent_expression_p (TREE_OPERAND (expression, 1)));
case CALL_EXPR:
- /* A CALL_EXPR is value-dependent if any argument is
- value-dependent. Why do we have to handle CALL_EXPRs in this
- function at all? First, some function calls, those for which
- value_dependent_expression_p is true, man appear in constant
- expressions. Second, there appear to be bugs which result in
- other CALL_EXPRs reaching this point. */
- {
- tree function = TREE_OPERAND (expression, 0);
- tree args = TREE_OPERAND (expression, 1);
-
- if (value_dependent_expression_p (function))
- return true;
-
- if (! args)
- return false;
-
- if (TREE_CODE (args) == TREE_LIST)
- {
- for (; args; args = TREE_CHAIN (args))
- if (value_dependent_expression_p (TREE_VALUE (args)))
- return true;
- return false;
- }
-
- return value_dependent_expression_p (args);
- }
+ /* A CALL_EXPR may appear in a constant expression if it is a
+ call to a builtin function, e.g., __builtin_constant_p. All
+ such calls are value-dependent. */
+ return true;
default:
/* A constant expression is value-dependent if any subexpression is
Index: gcc/cp/semantics.c
===================================================================
--- gcc/cp/semantics.c (revision 114635)
+++ gcc/cp/semantics.c (working copy)
@@ -3834,24 +3834,41 @@ finish_omp_for (location_t locus, tree d
void
finish_omp_atomic (enum tree_code code, tree lhs, tree rhs)
{
- /* If either of the operands are dependent, we can't do semantic
- processing yet. Stuff the values away for now. We cheat a bit
- and use the same tree code for this, even though the operands
- are of totally different form, thus we need to remember which
- statements are which, thus the lang_flag bit. */
- /* ??? We ought to be using type_dependent_expression_p, but the
- invocation of build_modify_expr in c_finish_omp_atomic can result
- in the creation of CONVERT_EXPRs, which are not handled by
- tsubst_copy_and_build. */
- if (uses_template_parms (lhs) || uses_template_parms (rhs))
+ tree orig_lhs;
+ tree orig_rhs;
+ bool dependent_p;
+ tree stmt;
+
+ orig_lhs = lhs;
+ orig_rhs = rhs;
+ dependent_p = false;
+ stmt = NULL_TREE;
+
+ /* Even in a template, we can detect invalid uses of the atomic
+ pragma if neither LHS nor RHS is type-dependent. */
+ if (processing_template_decl)
{
- tree stmt = build2 (OMP_ATOMIC, void_type_node, lhs, rhs);
+ dependent_p = (type_dependent_expression_p (lhs)
+ || type_dependent_expression_p (rhs));
+ if (!dependent_p)
+ {
+ lhs = build_non_dependent_expr (lhs);
+ rhs = build_non_dependent_expr (rhs);
+ }
+ }
+ if (!dependent_p)
+ {
+ stmt = c_finish_omp_atomic (code, lhs, rhs);
+ if (stmt == error_mark_node)
+ return;
+ }
+ if (processing_template_decl)
+ {
+ stmt = build2 (OMP_ATOMIC, void_type_node, orig_lhs, orig_rhs);
OMP_ATOMIC_DEPENDENT_P (stmt) = 1;
OMP_ATOMIC_CODE (stmt) = code;
- add_stmt (stmt);
}
- else
- c_finish_omp_atomic (code, lhs, rhs);
+ add_stmt (stmt);
}
void
Index: gcc/testsuite/g++.dg/gomp/tpl-atomic-2.C
===================================================================
--- gcc/testsuite/g++.dg/gomp/tpl-atomic-2.C (revision 114635)
+++ gcc/testsuite/g++.dg/gomp/tpl-atomic-2.C (working copy)
@@ -17,11 +17,10 @@ template<typename T> void f2(float *f)
}
// Here the rhs is dependent, but not type dependent.
-// ??? Fails. See the comment in finish_omp_atomic.
template<typename T> void f3(float *f)
{
#pragma omp atomic
- *f |= sizeof (T); // { dg-error "invalid|evaluation" "" { xfail *-*-* } }
+ *f |= sizeof (T); // { dg-error "invalid|evaluation" }
}
// And the converse, no error here because we're never fed a T.
Index: gcc/testsuite/g++.dg/template/builtin1.C
===================================================================
--- gcc/testsuite/g++.dg/template/builtin1.C (revision 0)
+++ gcc/testsuite/g++.dg/template/builtin1.C (revision 0)
@@ -0,0 +1,11 @@
+// PR c++/26559
+
+template<bool> struct cond;
+
+template<int> struct S {
+ void f(int i) {
+ cond<__builtin_constant_p(i)>();
+ }
+};
+
+S<1> s;
More information about the Gcc-patches
mailing list