On 07/13/2015 06:43 AM, Michael Matz wrote:
> This also hints at other problems (which might not actually occur in the
> case at hand, but still): the contents of DECL_VALUE_EXPR is the "real"
> thing containing the value of a decl (i.e. a decl having a value-expr
> doesn't itself occur in the code anymore), be it a decl itself, or some
> expression (which might also refer to decls). Now, in PR 66714 you
> analyzed that one of those D* was removed from the function, which should
> have happened only because no code referred to anymore, i.e. D* was also
> rewritten to some other D'* (if it weren't rewritten and D* was referred
> to in code, you would have created a miscompilation). At that point also
> the DECL_VALUE_EXPRs need to be rewritten to refer to D'*, not to D*
> anymore.
The attached patch does just that; it teaches
replace_block_vars_by_duplicates to replace the decls inside the
value-exprs with a duplicate too. It's kind of messy though. At the
moment I'm only considering VAR_DECL, PARM_DECL, RESULT_DECL, ADDR_EXPR,
ARRAY_REF, COMPONENT_REF, CONVERT_EXPR, NOP_EXPR, INDIRECT_REF and
MEM_REFs. I suspect that I may be missing some, but these are the only
ones that were triggered gcc_unreachable during testing.
As Tom mentioned in PR66714, this bug is present on trunk, specifically
in code using omp targets. Is this patch OK for trunk? I bootstrapped
and tested on x86_64-linux-gnu.
Cesar
2015-07-22 Cesar Philippidis <ce...@codesourcery.com>
Tom de Vries <vr...@codesourcery.com>
gcc/
* tree-cfg.c (replace_by_duplicate_decl_value_expr): New function.
(replace_block_vars_by_duplicates): Ensure that value expr decls
have been copied usign replace_by_duplicate_decl_value_expr.
libgomp/
* testsuite/libgomp.c/pr66714.c: New file.
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index fde7fbc..15cb122 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -6439,6 +6439,99 @@ replace_by_duplicate_decl (tree *tp, hash_map<tree, tree> *vars_map,
*tp = new_t;
}
+/* Replaces the value expression *TP with a duplicate (belonging to function
+ TO_CONTEXT). The duplicates are recorded in VARS_MAP. */
+
+static void
+replace_by_duplicate_decl_value_expr (tree *tp,
+ hash_map<tree, tree> *vars_map,
+ tree to_context)
+{
+ tree x = *tp;
+
+ switch (TREE_CODE (*tp))
+ {
+ case VAR_DECL:
+ case PARM_DECL:
+ case RESULT_DECL:
+ replace_by_duplicate_decl (tp, vars_map, to_context);
+ break;
+ case ADDR_EXPR:
+ {
+ tree expr = TREE_OPERAND (x, 0);
+
+ replace_by_duplicate_decl_value_expr (&expr, vars_map, to_context);
+ *tp = build1 (ADDR_EXPR, TREE_TYPE (x), expr);
+ }
+ break;
+ case ARRAY_REF:
+ {
+ tree array = TREE_OPERAND (x, 0);
+ tree index = TREE_OPERAND (x, 1);
+ tree arg2 = TREE_OPERAND (x, 2);
+ tree arg3 = TREE_OPERAND (x, 3);
+
+ replace_by_duplicate_decl (&array, vars_map, to_context);
+ replace_by_duplicate_decl (&index, vars_map, to_context);
+
+ *tp = build4 (ARRAY_REF, TREE_TYPE (x), array, index,
+ arg2, arg3);
+ }
+ break;
+ case COMPONENT_REF:
+ {
+ tree component = TREE_OPERAND (x, 0);
+ tree field = TREE_OPERAND (x, 1);
+ tree ref;
+
+ /* Components may be MEM_REFs. */
+ replace_by_duplicate_decl_value_expr (&component, vars_map,
+ to_context);
+ ref = build3 (COMPONENT_REF, TREE_TYPE (field), component,
+ field, NULL);
+
+ if (TREE_THIS_VOLATILE (x))
+ TREE_THIS_VOLATILE (ref) |= 1;
+ if (TREE_READONLY (x))
+ TREE_READONLY (ref) |= 1;
+
+ *tp = ref;
+ }
+ break;
+ case CONVERT_EXPR:
+ case NOP_EXPR:
+ case INDIRECT_REF:
+ {
+ tree expr = TREE_OPERAND (x, 0);
+ tree decl;
+
+ if (CONVERT_EXPR_CODE_P (TREE_CODE (expr)))
+ decl = TREE_OPERAND (expr, 0);
+ else
+ decl = expr;
+
+ replace_by_duplicate_decl (&decl, vars_map, to_context);
+
+ if (CONVERT_EXPR_CODE_P (TREE_CODE (expr)))
+ expr = build1 (TREE_CODE (expr), TREE_TYPE (expr), decl);
+ else
+ expr = decl;
+
+ *tp = build_simple_mem_ref (expr);
+ }
+ break;
+ case MEM_REF:
+ {
+ tree mem = TREE_OPERAND (x, 0);
+
+ replace_by_duplicate_decl_value_expr (&mem, vars_map, to_context);
+ *tp = build_simple_mem_ref (mem);
+ }
+ break;
+ default:
+ gcc_unreachable ();
+ }
+}
/* Creates an ssa name in TO_CONTEXT equivalent to NAME.
VARS_MAP maps old ssa names and var_decls to the new ones. */
@@ -6916,7 +7009,11 @@ replace_block_vars_by_duplicates (tree block, hash_map<tree, tree> *vars_map,
{
if (TREE_CODE (*tp) == VAR_DECL && DECL_HAS_VALUE_EXPR_P (*tp))
{
- SET_DECL_VALUE_EXPR (t, DECL_VALUE_EXPR (*tp));
+ tree x = DECL_VALUE_EXPR (*tp);
+
+ replace_by_duplicate_decl_value_expr (&x, vars_map, to_context);
+
+ SET_DECL_VALUE_EXPR (t, x);
DECL_HAS_VALUE_EXPR_P (t) = 1;
}
DECL_CHAIN (t) = DECL_CHAIN (*tp);
diff --git a/libgomp/testsuite/libgomp.c/pr66714.c b/libgomp/testsuite/libgomp.c/pr66714.c
new file mode 100644
index 0000000..c9af4a9
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/pr66714.c
@@ -0,0 +1,17 @@
+/* { dg-do "compile" } */
+/* { dg-additional-options "--param ggc-min-expand=0" } */
+/* { dg-additional-options "--param ggc-min-heapsize=0" } */
+/* { dg-additional-options "-g" } */
+
+/* Minimized from on target-2.c. */
+
+void
+fn3 (int x)
+{
+ double b[3 * x];
+ int i;
+#pragma omp target
+#pragma omp parallel for
+ for (i = 0; i < x; i++)
+ b[i] += 1;
+}
