From patchwork Thu Apr 28 10:26:24 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Schwinge X-Patchwork-Id: 616122 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 3qwY0c5KXkz9t6m for ; Thu, 28 Apr 2016 20:26:56 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=bOZT9/Xk; 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:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; q=dns; s=default; b=YCb74Wiq/z/Bk0Uy PWdg13pRdwtVOyv1RxUNYvXvpmgNsPjmtO93KBJVJvBVNhg9SRY941xl53tt0cKu EF5Gpky6leNxQ3hLWZLMzpwB+5DWkol9YdfO5KY9zXUno2MMf78enW9kH2qIG+jb nHVR8zuSFxHhHuuzuKyka/GsKvQ= 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:from :to:cc:subject:in-reply-to:references:date:message-id :mime-version:content-type; s=default; bh=F6zYt0WygZxRFiI1cVgB62 2gvcY=; b=bOZT9/XkY5ZFnh3H01jKSDJj3uX/o0OBcfnGhs7umdA6U64Mhuq83l xra6qqESQXPu+lKEezfg5yZdLl0Gx3BJDlrj+IH3TmxqKjUYu0pnsuolHa6J0xK4 MkSFjg2EVjP/GIM2BW0Cdo0O1FlXvOD7Ow+7gVCswwcpQBUnV1Sg8= Received: (qmail 98173 invoked by alias); 28 Apr 2016 10:26:46 -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 98160 invoked by uid 89); 28 Apr 2016 10:26:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 spammy=4737, Pretend, 2106, 2778 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 28 Apr 2016 10:26:35 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1avj9T-0000kx-Tw from Thomas_Schwinge@mentor.com ; Thu, 28 Apr 2016 03:26:31 -0700 Received: from tftp-cs (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Thu, 28 Apr 2016 03:26:31 -0700 Received: by tftp-cs (Postfix, from userid 49978) id 063CBC229E; Thu, 28 Apr 2016 03:26:30 -0700 (PDT) From: Thomas Schwinge To: Bernd Schmidt , Nathan Sidwell , CC: Richard Biener Subject: Avoid NULL cfun ICE in gcc/config/nvptx/nvptx.c:nvptx_libcall_value (was: [PATCH] Fix PR70760) In-Reply-To: References: User-Agent: Notmuch/0.9-125-g4686d11 (http://notmuchmail.org) Emacs/24.5.1 (i586-pc-linux-gnu) Date: Thu, 28 Apr 2016 12:26:24 +0200 Message-ID: <87wpnixc4v.fsf@kepler.schwinge.homeip.net> MIME-Version: 1.0 Hi! Richard's r235511 changes (quoted below) cause certain nvptx offloading test cases to run into SIGSEGVs: [...] #4 0x0000000000d14193 in nvptx_libcall_value (mode=mode@entry=SImode) at [...]/source-gcc/gcc/config/nvptx/nvptx.c:489 #5 0x0000000000d17a20 in nvptx_function_value (type=0x7fc1fa359690, func=0x0, outgoing=) at [...]/source-gcc/gcc/config/nvptx/nvptx.c:512 #6 0x00000000006ba220 in hard_function_value (valtype=valtype@entry=0x7fc1fa359690, func=func@entry=0x0, fntype=fntype@entry=0x0, outgoing=outgoing@entry=0) at [...]/source-gcc/gcc/explow.c:1860 #7 0x000000000073b0fa in aggregate_value_p (exp=exp@entry=0x7fc1fa41a048, fntype=0x0) at [...]/source-gcc/gcc/function.c:2086 #8 0x0000000000bebc11 in find_func_aliases_for_call (t=0x1feac90, fn=0x7ffe448ca8a0) at [...]/source-gcc/gcc/tree-ssa-structalias.c:4644 #9 find_func_aliases (fn=fn@entry=0x7fc1fa43a540, origt=origt@entry=0x7fc1fa43a7e0) at [...]/source-gcc/gcc/tree-ssa-structalias.c:4737 #10 0x0000000000bf04eb in ipa_pta_execute () at [...]/source-gcc/gcc/tree-ssa-structalias.c:7787 #11 (anonymous namespace)::pass_ipa_pta::execute (this=) at [...]/source-gcc/gcc/tree-ssa-structalias.c:8035 #12 0x0000000000940bed in execute_one_pass (pass=pass@entry=0x1f43770) at [...]/source-gcc/gcc/passes.c:2348 #13 0x0000000000941972 in execute_ipa_pass_list (pass=0x1f43770) at [...]/source-gcc/gcc/passes.c:2778 #14 0x0000000000607f1f in symbol_table::compile (this=0x7fc1fa359000) at [...]/source-gcc/gcc/cgraphunit.c:2435 #15 0x000000000056ad48 in lto_main () at [...]/source-gcc/gcc/lto/lto.c:3328 #16 0x0000000000a065df in compile_file () at [...]/source-gcc/gcc/toplev.c:474 #17 0x000000000053753a in do_compile () at [...]/source-gcc/gcc/toplev.c:1998 #18 toplev::main (this=this@entry=0x7ffe448caba0, argc=argc@entry=18, argv=0x1f1eec0, argv@entry=0x7ffe448caca8) at [...]/source-gcc/gcc/toplev.c:2106 #19 0x00000000005391d7 in main (argc=18, argv=0x7ffe448caca8) at [...]/source-gcc/gcc/main.c:39 The immediate problem is that gcc/config/nvptx/nvptx.c:nvptx_libcall_value is called in a context where cfun is NULL, and it fails to handle that appropriately: (gdb) frame 4 #4 0x0000000000d14193 in nvptx_libcall_value (mode=mode@entry=SImode) at [...]/source-gcc/gcc/config/nvptx/nvptx.c:489 489 if (!cfun->machine->doing_call) (gdb) print cfun $1 = (function *) 0x0 Looking at the backtrace, I see that in frame 7, gcc/function.c:aggregate_value_p is called with a NULL fntype. This function is evidently prepared to handle that case, likewise for gcc/explow.c:hard_function_value. Does it thus follow that gcc/config/nvptx/nvptx.c:nvptx_function_value and/or gcc/config/nvptx/nvptx.c:nvptx_libcall_value need to be changed? Is something like the following sufficient (works in offloading testing, but feels a bit like just "treating the symptoms"); for instance, should this case rather be handled in gcc/config/nvptx/nvptx.c:nvptx_function_value already? For reference: On Wed, 27 Apr 2016 13:07:36 +0200 (CEST), Richard Biener wrote: > The following patch fixes an issue in IPA PTA regarding to handling > of DECL_BY_REFERENCE function results at the caller side. The issue > for the testcase in the PR is that we use the wrong function decl > to look for DECL_RESULT for calls that are an alias (which get > DECL_RESULT released). > > But the issue is deeper in that the code also does not handle > indirect calls correctly - to expose a testcase for this the > patch also enables optimistic handling of functions escaping > via their addresses, this is already handled fine after I added > code to parse global initializers correctly. > > LTO bootstrapped and tested on x86_64-unknown-linux-gnu with IPA PTA > enabled, inspected PTA result on the PRs testcase (I failed to create a > small reproducer). > > Bootstrap / regtest running on x86_64-unknown-linux-gnu. > > This is the trunk version of the fix, for the branch where the > issue was reported against I will refrain from handling address-taken > functions differently. > > I hope I deciphered enough of the calls handling to assess that > aggregate_value_p always matches DECL_BY_REFERENCE on DECL_RESULT. > IPA PTA needs to know the GIMPLE representation of the callees > DECL_RESULT (whether it's a pointer - at the caller side we > still see the non-reference LHS). And that needs to work for > indirect calls as well. > > Richard. > > 2016-04-27 Richard Biener > > PR ipa/70760 > * tree-ssa-structalias.c (find_func_aliases_for_call): Use > aggregate_value_p to determine if a function result is > returned by reference. > (ipa_pta_execute): Functions having their address taken are > not automatically nonlocal. > > * g++.dg/ipa/ipa-pta-2.C: New testcase. > * gcc.dg/ipa/ipa-pta-1.c: Adjust. > > Index: gcc/tree-ssa-structalias.c > =================================================================== > *** gcc/tree-ssa-structalias.c (revision 235478) > --- gcc/tree-ssa-structalias.c (working copy) > *************** find_func_aliases_for_call (struct funct > *** 4641,4652 **** > auto_vec lhsc; > struct constraint_expr rhs; > struct constraint_expr *lhsp; > > get_constraint_for (lhsop, &lhsc); > rhs = get_function_part_constraint (fi, fi_result); > ! if (fndecl > ! && DECL_RESULT (fndecl) > ! && DECL_BY_REFERENCE (DECL_RESULT (fndecl))) > { > auto_vec tem; > tem.quick_push (rhs); > --- 4737,4747 ---- > auto_vec lhsc; > struct constraint_expr rhs; > struct constraint_expr *lhsp; > + bool aggr_p = aggregate_value_p (lhsop, gimple_call_fntype (t)); > > get_constraint_for (lhsop, &lhsc); > rhs = get_function_part_constraint (fi, fi_result); > ! if (aggr_p) > { > auto_vec tem; > tem.quick_push (rhs); > *************** find_func_aliases_for_call (struct funct > *** 4656,4677 **** > } > FOR_EACH_VEC_ELT (lhsc, j, lhsp) > process_constraint (new_constraint (*lhsp, rhs)); > - } > > ! /* If we pass the result decl by reference, honor that. */ > ! if (lhsop > ! && fndecl > ! && DECL_RESULT (fndecl) > ! && DECL_BY_REFERENCE (DECL_RESULT (fndecl))) > ! { > ! struct constraint_expr lhs; > ! struct constraint_expr *rhsp; > > ! get_constraint_for_address_of (lhsop, &rhsc); > ! lhs = get_function_part_constraint (fi, fi_result); > ! FOR_EACH_VEC_ELT (rhsc, j, rhsp) > ! process_constraint (new_constraint (lhs, *rhsp)); > ! rhsc.truncate (0); > } > > /* If we use a static chain, pass it along. */ > --- 4751,4769 ---- > } > FOR_EACH_VEC_ELT (lhsc, j, lhsp) > process_constraint (new_constraint (*lhsp, rhs)); > > ! /* If we pass the result decl by reference, honor that. */ > ! if (aggr_p) > ! { > ! struct constraint_expr lhs; > ! struct constraint_expr *rhsp; > > ! get_constraint_for_address_of (lhsop, &rhsc); > ! lhs = get_function_part_constraint (fi, fi_result); > ! FOR_EACH_VEC_ELT (rhsc, j, rhsp) > ! process_constraint (new_constraint (lhs, *rhsp)); > ! rhsc.truncate (0); > ! } > } > > /* If we use a static chain, pass it along. */ > *************** ipa_pta_execute (void) > *** 7686,7715 **** > > gcc_assert (!node->clone_of); > > - /* When parallelizing a code region, we split the region off into a > - separate function, to be run by several threads in parallel. So for a > - function foo, we split off a region into a function > - foo._0 (void *foodata), and replace the region with some variant of a > - function call run_on_threads (&foo._0, data). The '&foo._0' sets the > - address_taken bit for function foo._0, which would make it non-local. > - But for the purpose of ipa-pta, we can regard the run_on_threads call > - as a local call foo._0 (data), so we ignore address_taken on nodes > - with parallelized_function set. > - Note: this is only safe, if foo and foo._0 are in the same lto > - partition. */ > - bool node_address_taken = ((node->parallelized_function > - && !node->used_from_other_partition) > - ? false > - : node->address_taken); > - > /* For externally visible or attribute used annotated functions use > local constraints for their arguments. > For local functions we see all callers and thus do not need initial > constraints for parameters. */ > bool nonlocal_p = (node->used_from_other_partition > || node->externally_visible > ! || node->force_output > ! || node_address_taken); > node->call_for_symbol_thunks_and_aliases (refered_from_nonlocal_fn, > &nonlocal_p, true); > > --- 7778,7790 ---- > > gcc_assert (!node->clone_of); > > /* For externally visible or attribute used annotated functions use > local constraints for their arguments. > For local functions we see all callers and thus do not need initial > constraints for parameters. */ > bool nonlocal_p = (node->used_from_other_partition > || node->externally_visible > ! || node->force_output); > node->call_for_symbol_thunks_and_aliases (refered_from_nonlocal_fn, > &nonlocal_p, true); > > Index: gcc/testsuite/g++.dg/ipa/ipa-pta-2.C > =================================================================== > *** gcc/testsuite/g++.dg/ipa/ipa-pta-2.C (revision 0) > --- gcc/testsuite/g++.dg/ipa/ipa-pta-2.C (working copy) > *************** > *** 0 **** > --- 1,37 ---- > + // { dg-do run } > + // { dg-options "-O2 -fipa-pta" } > + > + extern "C" void abort (void); > + > + struct Y { ~Y(); int i; }; > + > + Y::~Y () {} > + > + static Y __attribute__((noinline)) foo () > + { > + Y res; > + res.i = 3; > + return res; > + } > + > + static Y __attribute__((noinline)) bar () > + { > + Y res; > + res.i = 42; > + return res; > + } > + > + static Y (*fn) (); > + > + int a; > + int main() > + { > + if (a) > + fn = foo; > + else > + fn = bar; > + Y res = fn (); > + if (res.i != 42) > + abort (); > + return 0; > + } > Index: gcc/testsuite/gcc.dg/ipa/ipa-pta-1.c > =================================================================== > *** gcc/testsuite/gcc.dg/ipa/ipa-pta-1.c (revision 235478) > --- gcc/testsuite/gcc.dg/ipa/ipa-pta-1.c (working copy) > *************** int main() > *** 40,52 **** > } > > /* IPA PTA needs to handle indirect calls properly. Verify that > ! both bar and foo get a (and only a) in their arguments points-to sets. > ! ??? As bar and foo have their address taken there might be callers > ! not seen by IPA PTA (if the address escapes the unit which we only compute > ! during IPA PTA...). Thus the solution also includes NONLOCAL. */ > > /* { dg-final { scan-ipa-dump "fn_1 = { bar foo }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "bar.arg0 = { NONLOCAL a }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "bar.arg1 = { NONLOCAL a }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "foo.arg0 = { NONLOCAL a }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "foo.arg1 = { NONLOCAL a }" "pta2" } } */ > --- 40,49 ---- > } > > /* IPA PTA needs to handle indirect calls properly. Verify that > ! both bar and foo get a (and only a) in their arguments points-to sets. */ > > /* { dg-final { scan-ipa-dump "fn_1 = { bar foo }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "bar.arg0 = { a }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "bar.arg1 = { a }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "foo.arg0 = { a }" "pta2" } } */ > ! /* { dg-final { scan-ipa-dump "foo.arg1 = { a }" "pta2" } } */ Grüße Thomas --- gcc/config/nvptx/nvptx.c +++ gcc/config/nvptx/nvptx.c @@ -484,7 +484,7 @@ nvptx_strict_argument_naming (cumulative_args_t cum_v) static rtx nvptx_libcall_value (machine_mode mode, const_rtx) { - if (!cfun->machine->doing_call) + if (!cfun || !cfun->machine->doing_call) /* Pretend to return in a hard reg for early uses before pseudos can be generated. */ return gen_rtx_REG (mode, NVPTX_RETURN_REGNUM);