From patchwork Thu Nov 17 09:27:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chung-Lin Tang X-Patchwork-Id: 696038 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 3tKG4k1vJDz9t1Q for ; Thu, 17 Nov 2016 20:27:50 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="sk31Cl1I"; 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=ralJSKP6GL6Lav2pf BcF3cTc3HZt5BaEEpmletnbipYuNr2h+bvEH+7utqH0U5WmW/SslG8jG9rWIJWsI wX4h6DxktjP2xTmp2EFEriWyJmj818inZpZmT1ogafJG3EVz6BUlNHa/aC7aj4VJ KCrcZo5Vw2CkbGXTvz0fojgsds= 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 :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=Z2XJqzbtrsylwINvtRTgdfn UzP4=; b=sk31Cl1I+UngqnBIaRyj+Nhjssy46R41CcwPRQiE9avMHItd+tNWWOH d/eE+Evcod330uWods9PTDXw6zzKnqrLv7HJ4QNb+3cuj+8dTAdleFVm2KsGC/6E O9rsxS/vB0/chv7Fuq5ZaGxgHaJLyzkocfCzxJUmQHKWP1hYvSKg= Received: (qmail 76364 invoked by alias); 17 Nov 2016 09:27:42 -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 75318 invoked by uid 89); 17 Nov 2016 09:27:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 spammy=165410, 1654, 10, tree_value, TREE_VALUE X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Nov 2016 09:27:38 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1c7Iyj-0000hG-Fm from ChungLin_Tang@mentor.com ; Thu, 17 Nov 2016 01:27:33 -0800 Received: from svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Thu, 17 Nov 2016 01:27:31 -0800 Received: from [0.0.0.0] (147.34.91.1) by svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) with Microsoft SMTP Server (TLS) id 15.0.1210.3 via Frontend Transport; Thu, 17 Nov 2016 01:27:25 -0800 Subject: Re: [Patch 1/5] OpenACC tile clause support, OMP_CLAUSE_TILE adjustments To: Jakub Jelinek References: <9f4c0d8a-b26f-573e-0746-395dba321c8f@mentor.com> <20161111093803.GU3541@tucnak.redhat.com> CC: gcc-patches , Nathan Sidwell , Cesar Philippidis From: Chung-Lin Tang Message-ID: <7fb21d78-1a4f-986d-6c6a-069c8acbad7a@mentor.com> Date: Thu, 17 Nov 2016 17:27:14 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161111093803.GU3541@tucnak.redhat.com> On 2016/11/11 5:38 PM, Jakub Jelinek wrote: > Hi! > > On Thu, Nov 10, 2016 at 06:44:52PM +0800, Chung-Lin Tang wrote: > > Above this it is fine. > >> @@ -9388,10 +9373,23 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p) >> (OMP_FOR_INIT (for_stmt)) >> * 2); >> } >> - int collapse = 1; >> - c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_COLLAPSE); >> - if (c) >> - collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c)); >> + int collapse = 0; >> + /* Find the first of COLLAPSE or TILE. */ >> + for (c = OMP_FOR_CLAUSES (for_stmt); c; c = TREE_CHAIN (c)) >> + if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_COLLAPSE) >> + { >> + collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c)); >> + if (collapse == 1) >> + /* Not really collapsing. */ >> + collapse = 0; >> + break; >> + } >> + else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_TILE) >> + { >> + collapse = list_length (OMP_CLAUSE_TILE_LIST (c)); >> + break; >> + } > > I don't really like this, especially pretending collapse(1) or lack > of collapse clause e.g. on OpenMP construct is collapse(0). > I'd keep what it does, i.e. > int collapse = 1; > c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_COLLAPSE); > if (c) > collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c)); > and in the first switch in gimplify_omp_for you can: > case OACC_LOOP: > ort = ORT_ACC; > c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_TILE); > if (c) > tile = list_length (OMP_CLAUSE_TILE_LIST (c)); > break; > and then just use tile != 0 or whatever || with collapse > 1 where needed. > > > + >> for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)); i++) >> { >> t = TREE_VEC_ELT (OMP_FOR_INIT (for_stmt), i); >> @@ -9807,7 +9805,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p) >> OMP_CLAUSE_LINEAR_STEP (c2) = OMP_CLAUSE_LINEAR_STEP (c); >> } >> >> - if ((var != decl || collapse > 1) && orig_for_stmt == for_stmt) >> + if ((var != decl || collapse) && orig_for_stmt == for_stmt) >> { >> for (c = OMP_FOR_CLAUSES (for_stmt); c ; c = OMP_CLAUSE_CHAIN (c)) >> if (((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE > > Like here. > >> @@ -9817,7 +9815,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p) >> && OMP_CLAUSE_LINEAR_GIMPLE_SEQ (c) == NULL)) >> && OMP_CLAUSE_DECL (c) == decl) >> { >> - if (is_doacross && (collapse == 1 || i >= collapse)) >> + if (is_doacross && (!collapse || i >= collapse)) >> t = var; >> else >> { > > And not here. You don't really have doacross loops in OpenACC, do you? > > Jakub > Hi Jakub, I've updated the patch as you suggested. Here's the v2 of the first patch, mainly gimplify.c adjusted. About the ChangeLog issues, I'll make sure the final committed diffs will solve them. Thanks, Chung-Lin Index: tree.c =================================================================== --- tree.c (revision 241809) +++ tree.c (working copy) @@ -327,7 +327,7 @@ unsigned const char omp_clause_num_ops[] = 1, /* OMP_CLAUSE_NUM_GANGS */ 1, /* OMP_CLAUSE_NUM_WORKERS */ 1, /* OMP_CLAUSE_VECTOR_LENGTH */ - 1, /* OMP_CLAUSE_TILE */ + 3, /* OMP_CLAUSE_TILE */ 2, /* OMP_CLAUSE__GRIDDIM_ */ }; Index: tree.h =================================================================== --- tree.h (revision 241809) +++ tree.h (working copy) @@ -1654,6 +1654,10 @@ extern void protected_set_expr_location (tree, loc #define OMP_CLAUSE_TILE_LIST(NODE) \ OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_TILE), 0) +#define OMP_CLAUSE_TILE_ITERVAR(NODE) \ + OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_TILE), 1) +#define OMP_CLAUSE_TILE_COUNT(NODE) \ + OMP_CLAUSE_OPERAND (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_TILE), 2) #define OMP_CLAUSE__GRIDDIM__DIMENSION(NODE) \ (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE__GRIDDIM_)\ Index: tree-nested.c =================================================================== --- tree-nested.c (revision 241809) +++ tree-nested.c (working copy) @@ -1274,6 +1274,7 @@ convert_nonlocal_omp_clauses (tree *pclauses, stru case OMP_CLAUSE_DEFAULT: case OMP_CLAUSE_COPYIN: case OMP_CLAUSE_COLLAPSE: + case OMP_CLAUSE_TILE: case OMP_CLAUSE_UNTIED: case OMP_CLAUSE_MERGEABLE: case OMP_CLAUSE_PROC_BIND: @@ -1286,8 +1287,6 @@ convert_nonlocal_omp_clauses (tree *pclauses, stru case OMP_CLAUSE_AUTO: break; - /* OpenACC tile clauses are discarded during gimplification. */ - case OMP_CLAUSE_TILE: /* The following clause belongs to the OpenACC cache directive, which is discarded during gimplification. */ case OMP_CLAUSE__CACHE_: @@ -1982,6 +1981,7 @@ convert_local_omp_clauses (tree *pclauses, struct case OMP_CLAUSE_DEFAULT: case OMP_CLAUSE_COPYIN: case OMP_CLAUSE_COLLAPSE: + case OMP_CLAUSE_TILE: case OMP_CLAUSE_UNTIED: case OMP_CLAUSE_MERGEABLE: case OMP_CLAUSE_PROC_BIND: @@ -1994,8 +1994,6 @@ convert_local_omp_clauses (tree *pclauses, struct case OMP_CLAUSE_AUTO: break; - /* OpenACC tile clauses are discarded during gimplification. */ - case OMP_CLAUSE_TILE: /* The following clause belongs to the OpenACC cache directive, which is discarded during gimplification. */ case OMP_CLAUSE__CACHE_: Index: gimplify.c =================================================================== --- gimplify.c (revision 241809) +++ gimplify.c (working copy) @@ -8138,20 +8138,11 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_se remove = true; break; - case OMP_CLAUSE_TILE: - for (tree list = OMP_CLAUSE_TILE_LIST (c); !remove && list; - list = TREE_CHAIN (list)) - { - if (gimplify_expr (&TREE_VALUE (list), pre_p, NULL, - is_gimple_val, fb_rvalue) == GS_ERROR) - remove = true; - } - break; - case OMP_CLAUSE_NOWAIT: case OMP_CLAUSE_ORDERED: case OMP_CLAUSE_UNTIED: case OMP_CLAUSE_COLLAPSE: + case OMP_CLAUSE_TILE: case OMP_CLAUSE_AUTO: case OMP_CLAUSE_SEQ: case OMP_CLAUSE_INDEPENDENT: @@ -8927,13 +8918,7 @@ gimplify_adjust_omp_clauses (gimple_seq *pre_p, gi case OMP_CLAUSE_VECTOR: case OMP_CLAUSE_AUTO: case OMP_CLAUSE_SEQ: - break; - case OMP_CLAUSE_TILE: - /* We're not yet making use of the information provided by OpenACC - tile clauses. Discard these here, to simplify later middle end - processing. */ - remove = true; break; default: @@ -9388,10 +9373,13 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p) (OMP_FOR_INIT (for_stmt)) * 2); } - int collapse = 1; + int collapse = 1, tile = 0; c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_COLLAPSE); if (c) collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (c)); + c = find_omp_clause (OMP_FOR_CLAUSES (for_stmt), OMP_CLAUSE_TILE); + if (c) + tile = list_length (OMP_CLAUSE_TILE_LIST (c)); for (i = 0; i < TREE_VEC_LENGTH (OMP_FOR_INIT (for_stmt)); i++) { t = TREE_VEC_ELT (OMP_FOR_INIT (for_stmt), i); @@ -9807,7 +9795,7 @@ gimplify_omp_for (tree *expr_p, gimple_seq *pre_p) OMP_CLAUSE_LINEAR_STEP (c2) = OMP_CLAUSE_LINEAR_STEP (c); } - if ((var != decl || collapse > 1) && orig_for_stmt == for_stmt) + if ((var != decl || collapse > 1 || tile) && orig_for_stmt == for_stmt) { for (c = OMP_FOR_CLAUSES (for_stmt); c ; c = OMP_CLAUSE_CHAIN (c)) if (((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE