This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] PR66714 -- Re: Re: [RFC] two-phase marking in gt_cleare_cache
- From: Cesar Philippidis <cesar_philippidis at mentor dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Michael Matz <matz at suse dot de>, Tom de Vries <Tom_deVries at mentor dot com>, Richard Biener <richard dot guenther at gmail dot com>, "gcc-patches at gnu dot org" <gcc-patches at gnu dot org>
- Date: Fri, 24 Jul 2015 07:21:07 -0700
- Subject: Re: [patch] PR66714 -- Re: Re: [RFC] two-phase marking in gt_cleare_cache
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 20 dot 1507131504490 dot 23227 at wotan dot suse dot de> <55B10652 dot 8050304 at mentor dot com> <20150723153241 dot GD1780 at tucnak dot redhat dot com> <55B16435 dot 8080204 at mentor dot com> <20150723221111 dot GG1780 at tucnak dot redhat dot com>
On 07/23/2015 03:11 PM, Jakub Jelinek wrote:
> On Thu, Jul 23, 2015 at 03:01:25PM -0700, Cesar Philippidis wrote:
>> On 07/23/2015 08:32 AM, Jakub Jelinek wrote:
>>> On Thu, Jul 23, 2015 at 08:20:50AM -0700, Cesar Philippidis wrote:
>>>> 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.
>>>
>>> Ugh, that looks ugly, why do we have all the tree walkers?
>>> I'd unshare_expr the value expr first, you really don't want to share
>>> it anyway, and then just walk_tree and find all the decls in there
>>> (with *walk_subtrees on types and perhaps something else too) and for them
>>> replace_by_duplicate_decl (tp, vars_map, to_context);
>>
>> Something like the attached patch? Why do TREE_TYPEs need special handling?
>
> They can have decls in various places like TYPE_SIZE_UNIT, TYPE_SIZE, the
> bounds of TYPE_DOMAIN etc. and I believe you generally don't want to replace
> those. Plus you risk infinite recursion then (unless walk_tree_without_duplicates).
> Most walk_tree callbacks just do something like
> if (IS_TYPE_OR_DECL_P (*tp))
> *walk_subtrees = 0;
This patch the check for IS_TYPE_OF_DECL_P in this patch. Is this ok for
trunk?
Cesar
2015-07-24 Cesar Philippidis <cesar@codesourcery.com>
gcc/
* tree-cfg.c (struct replace_decls_d): New struct.
(replace_block_vars_by_duplicates_1): New function.
(replace_block_vars_by_duplicates): Use it to replace the decls
in the value exprs by duplicates.
libgomp/
* testsuite/libgomp.c/pr66714.c: New test.
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index fde7fbc..cb9fe6d 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -70,6 +70,7 @@ along with GCC; see the file COPYING3. If not see
#include "omp-low.h"
#include "tree-cfgcleanup.h"
#include "wide-int-print.h"
+#include "gimplify.h"
/* This file contains functions for building the Control Flow Graph (CFG)
for a function tree. */
@@ -108,6 +109,13 @@ struct cfg_stats_d
static struct cfg_stats_d cfg_stats;
+/* Data to pass to replace_block_vars_by_duplicates_1. */
+struct replace_decls_d
+{
+ hash_map<tree, tree> *vars_map;
+ tree to_context;
+};
+
/* Hash table to store last discriminator assigned for each locus. */
struct locus_discrim_map
{
@@ -6897,6 +6905,31 @@ new_label_mapper (tree decl, void *data)
return m->to;
}
+/* Tree walker to replace the decls used inside value expressions by
+ duplicates. */
+
+static tree
+replace_block_vars_by_duplicates_1 (tree *tp, int *walk_subtrees, void *data)
+{
+ struct replace_decls_d *rd = (struct replace_decls_d *)data;
+
+ switch (TREE_CODE (*tp))
+ {
+ case VAR_DECL:
+ case PARM_DECL:
+ case RESULT_DECL:
+ replace_by_duplicate_decl (tp, rd->vars_map, rd->to_context);
+ break;
+ default:
+ break;
+ }
+
+ if (IS_TYPE_OR_DECL_P (*tp))
+ *walk_subtrees = false;
+
+ return NULL;
+}
+
/* Change DECL_CONTEXT of all BLOCK_VARS in block, including
subblocks. */
@@ -6916,7 +6949,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);
+ struct replace_decls_d rd = { vars_map, to_context };
+ unshare_expr (x);
+ walk_tree (&x, replace_block_vars_by_duplicates_1, &rd, NULL);
+ 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;
+}