From patchwork Thu Dec 3 18:26:20 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 552412 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BFBE41401DA for ; Fri, 4 Dec 2015 05:26:32 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=yjG3CaZW; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=ZuHaOxQadKsoF61hcXmumiAKGFXLBa3/Tl1MaH29vdWsT2pUaqoLZ 1tMt07E5zi5O6H3EQmAoEkIHYWgKc3U/RmImY/5UWZQWSnsseBw93mxobSnz6wkW NIQ0Fyz2bF+cEbxB+T1XHOnQDQ+eO2W7S4G4SCXfAhHcXE54bZG2bU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=MjbxTyP1SACzrctunENpK4g9FM8=; b=yjG3CaZWTgVb35P9S74O bLPE0t6kCpqNE+2ylf2lifcF+n1bhIdaw2UjRDDu8FB6yxg0YxE1JmjljvsLUVN5 pxwt0aZ1S0g4dSvTjWIx9fk4VyS0kmxVcrAP0YyETMPjezz3JViNGn9fZ9bhFF9k 0W80pOocmLZjVE7gURHRmHY= Received: (qmail 43002 invoked by alias); 3 Dec 2015 18:26:26 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 42986 invoked by uid 89); 3 Dec 2015 18:26:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Thu, 03 Dec 2015 18:26:23 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 61A2BADE4 for ; Thu, 3 Dec 2015 18:26:20 +0000 (UTC) Date: Thu, 3 Dec 2015 19:26:20 +0100 From: Martin Jambor To: GCC Patches Subject: [hsa] Make copy_gimple_seq_and_replace_locals copy seqs in omp clauses Message-ID: <20151203182619.GH3155@virgil.suse.cz> Mail-Followup-To: GCC Patches MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes Hi, this is a fix to the last "last" ICE of the hsa branch. THe problem turned out not to be in the gridification itself but, depending your point of view, in the gimple and tree walking infrastructure or in function copy_gimple_seq_and_replace_locals from tree-inline.c on which hsa gridification relies. The issue is that in between gimplification and omplow pass, there can be gimple sequences attached to OMP_CLAUSE trees that are attached to omp statements and that are neither copied by gimple_seq_copy nor walked by walk_gimple_seq. While the correct solution would probably be to extend tree and gimple walkers to handle them, that would be a big change. I have talked with Jakub about this yesterday on the IRC and he suggested that I enhance the internal walkers of copy_gimple_seq_and_replace_locals deal with this situation. Even though that leaves gimple_seq_copy, walk_gimple_seq and other to be technically incorrect, that is what I have done in the patch below, which fixes my last ICEs and which I have already committed to the branch. Any feedback is of course very much appreciated, Martin 2015-12-03 Martin Jambor * tree-inline.c (duplicate_remap_omp_clause_seq): New function. (replace_locals_op): Duplicate gimple sequences in OMP clauses. --- gcc/tree-inline.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index ebab189..15141dc 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -5116,6 +5116,8 @@ mark_local_labels_stmt (gimple_stmt_iterator *gsip, return NULL_TREE; } +static gimple_seq duplicate_remap_omp_clause_seq (gimple_seq seq, + struct walk_stmt_info *wi); /* Called via walk_gimple_seq by copy_gimple_seq_and_replace_local. Using the splay_tree pointed to by ST (which is really a `splay_tree'), @@ -5160,6 +5162,35 @@ replace_locals_op (tree *tp, int *walk_subtrees, void *data) TREE_OPERAND (expr, 3) = NULL_TREE; } } + else if (TREE_CODE (expr) == OMP_CLAUSE) + { + /* Before the omplower pass completes, some OMP clauses can contain + sequences that are neither copied by gimple_seq_copy nor walked by + walk_gimple_seq. To make copy_gimple_seq_and_replace_locals work even + in those situations, we have to copy and process them explicitely. */ + + if (OMP_CLAUSE_CODE (expr) == OMP_CLAUSE_LASTPRIVATE) + { + gimple_seq seq = OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ (expr); + seq = duplicate_remap_omp_clause_seq (seq, wi); + OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ (expr) = seq; + } + else if (OMP_CLAUSE_CODE (expr) == OMP_CLAUSE_LINEAR) + { + gimple_seq seq = OMP_CLAUSE_LINEAR_GIMPLE_SEQ (expr); + seq = duplicate_remap_omp_clause_seq (seq, wi); + OMP_CLAUSE_LINEAR_GIMPLE_SEQ (expr) = seq; + } + else if (OMP_CLAUSE_CODE (expr) == OMP_CLAUSE_REDUCTION) + { + gimple_seq seq = OMP_CLAUSE_REDUCTION_GIMPLE_INIT (expr); + seq = duplicate_remap_omp_clause_seq (seq, wi); + OMP_CLAUSE_REDUCTION_GIMPLE_INIT (expr) = seq; + seq = OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (expr); + seq = duplicate_remap_omp_clause_seq (seq, wi); + OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (expr) = seq; + } + } /* Keep iterating. */ return NULL_TREE; @@ -5200,6 +5231,18 @@ replace_locals_stmt (gimple_stmt_iterator *gsip, return NULL_TREE; } +/* Create a copy of SEQ and remap all decls in it. */ + +static gimple_seq +duplicate_remap_omp_clause_seq (gimple_seq seq, struct walk_stmt_info *wi) +{ + /* If there are any labels in OMP sequences, they can be only referred to in + the sequence itself and therefore we can do both here. */ + walk_gimple_seq (seq, mark_local_labels_stmt, NULL, wi); + gimple_seq copy = gimple_seq_copy (seq); + walk_gimple_seq (copy, replace_locals_stmt, replace_locals_op, wi); + return copy; +} /* Copies everything in SEQ and replaces variables and labels local to current_function_decl. */