From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25467 invoked by alias); 26 Mar 2018 10:30:46 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 25414 invoked by uid 89); 26 Mar 2018 10:30:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=walk 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; Mon, 26 Mar 2018 10:30:38 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-MBX-04.mgc.mentorg.com) by relay1.mentorg.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-SHA384:256) id 1f0POd-0004Ms-UL from Tom_deVries@mentor.com ; Mon, 26 Mar 2018 03:30:35 -0700 Received: from [172.30.73.224] (137.202.0.87) by SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Mon, 26 Mar 2018 11:30:32 +0100 Subject: Re: [PATCH] Fix ICE for static vars in offloaded functions To: Richard Biener CC: Jakub Jelinek , GCC Patches References: <2299637a-3124-8609-e68d-225703f16abe@mentor.com> <20180307132516.GE5867@tucnak> <445e16ae-3a42-89f6-345f-6dd232f6c6e0@mentor.com> From: Tom de Vries Message-ID: <2c7e87a9-2723-27f9-1cb4-8099e5631428@mentor.com> Date: Mon, 26 Mar 2018 11:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) X-SW-Source: 2018-03/txt/msg01379.txt.bz2 On 03/07/2018 04:01 PM, Richard Biener wrote: > On Wed, 7 Mar 2018, Tom de Vries wrote: > >> On 03/07/2018 02:29 PM, Richard Biener wrote: >>> On Wed, 7 Mar 2018, Jakub Jelinek wrote: >>> >>>> On Wed, Mar 07, 2018 at 02:20:26PM +0100, Tom de Vries wrote: >>>>> Fix ICE for static vars in offloaded functions >>>>> >>>>> 2018-03-06 Tom de Vries >>>>> >>>>> PR lto/84592 >>>>> * varpool.c (varpool_node::get_create): Mark static variables in >>>>> offloaded functions as offloadable. >>>>> >>>>> * testsuite/libgomp.c/pr84592-2.c: New test. >>>>> * testsuite/libgomp.c/pr84592.c: New test. >>>>> * testsuite/libgomp.oacc-c-c++-common/pr84592-3.c: New test. >>>> >>>> Ok, thanks >>> >>> + bool in_offload_func >>> + = (cfun >>> + && TREE_STATIC (decl) >>> + && (lookup_attribute ("omp target entr >>> >>> I think you want to use decl_function_context (decl) here, >>> not rely on magic cfun being set. The whole varpool.c file >>> doesn't mention cfun yet and you shoudln't either. >>> >> >> decl_function_context (decl) returns main: >> ... >> (gdb) call debug_generic_expr (decl) >> test >> (gdb) call decl_function_context (decl) >> $2 = (tree_node *) 0x7ffff6978c00 >> (gdb) call debug_generic_expr ($2) >> main >> ... >> while the function annotated as being an offload function is main._omp_fn.0. > > Well, that's because the static isn't duplicated (it can't be) so it > retains the original context. > [ Actually the static is duplicated in replace_by_duplicate_decl, but the statements using it are not rewritten to use the duplicate, so indeed, effectively it's not duplicated. ] >> The varpool_node::get_create is called during cgraph_edge::rebuild_edges here >> in expand_omp_target: > > But at this point it's not created but just looked up, right? > No, the varpool_node is created at that point. > I think the fix is to mark the decl as offloaded when we walk the IL > of the outlined function. The current point looks like a hack. > OK, I'll try to find a better fix location. Thanks, - Tom > Richard. > >> ... >> 7087 /* Fix the callgraph edges for child_cfun. Those for cfun will >> be >> 7088 fixed in a following pass. */ >> 7089 push_cfun (child_cfun); >> 7090 if (need_asm) >> 7091 assign_assembler_name_if_needed (child_fn); >> 7092 cgraph_edge::rebuild_edges (); >> ... >> >> Thanks, >> - Tom >> >> >