From patchwork Thu Dec 1 13:05:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prathamesh Kulkarni X-Patchwork-Id: 86054 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp680841qgi; Thu, 1 Dec 2016 05:06:06 -0800 (PST) X-Received: by 10.98.84.68 with SMTP id i65mr38839109pfb.133.1480597566425; Thu, 01 Dec 2016 05:06:06 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id y92si149362plb.54.2016.12.01.05.06.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 01 Dec 2016 05:06:06 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-443203-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-443203-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-443203-patch=linaro.org@gcc.gnu.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; q=dns; s=default; b=WsyiVzuyPfuSi2S AQPcl/Cthy3R4s+nYFO40SdO5Aub46GNsQdhRjzcQmMMx0fxZG/o5ETpbNR0fvhO ajN/K8NS8ZhlrWNEGQopeuNSLVz8+KL3x/krIA+COZooV7i3Jx6I3Edg28yzwm// weJ5UwU9S+nSzro0dH4AsYWz8Qyw= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; s=default; bh=7uSQQglTAwc0FkqI/8tvO lwFdNU=; b=AHNWigPbMIYNRH6XJA0a+oTy/9O/OzWxVa42m/KYEWXcJS5jazVvX +himOATpShb0iv150k7cYM1bwZn7MRU2TMikueT8iTASKy+V8SMtQDq8+kQDwI4u Hsk7/IqjS+v/VjwZO//pDHTP0AcFll7GikM0U5BnnySYsw3Igi2bRU= Received: (qmail 38515 invoked by alias); 1 Dec 2016 13:05:50 -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 37673 invoked by uid 89); 1 Dec 2016 13:05:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.0 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=abb, 4086, 4016, ERF_RETURNS_ARG X-HELO: mail-io0-f177.google.com Received: from mail-io0-f177.google.com (HELO mail-io0-f177.google.com) (209.85.223.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Dec 2016 13:05:39 +0000 Received: by mail-io0-f177.google.com with SMTP id c21so387474710ioj.1 for ; Thu, 01 Dec 2016 05:05:39 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=ijOd3fGzcMsfCdcjmY35bt/SwPUafBime/RA6Jr7qbg=; b=EwW1PCOPIKbgdG3Q72a0SebZ+jQ7nyOUufX7zZ8lRJIEyvtis3WRZUUjiZ98Mi2/K4 j2YOFWEgPN2+Y7I/kp/S1oaB6vZA6NebCWojIUgf3EOyNJC4Na1xdLrHVTcdsl2BC27d UgX9Qyjl4SLl9S3aYmHYqkySBsWF+AkuWOWer1ZzcoRchjAGbuP5SjbRjStY6ZYpAQDM a4Piyn+pwwFED5dd8BaAPcvzPW1cHVkeatCjsOpOOAcl1Lt7iYlsiHxs8FItuTXZHsvO +IfopRxbOSoGhf7+RjHqjtUQ1oNVSKTvpgj9snGAzA99bbRgp4DvEF0rFL3zLrMwuzJf 0sbQ== X-Gm-Message-State: AKaTC03i6jImPsirMilSOQkHFfoA6hP2BKlQTMI2oDpBIlAYFqQhBsnJqgawItKrXsvCPgjBHGpEbq5MbDaTIWkn X-Received: by 10.36.215.197 with SMTP id y188mr33275792itg.71.1480597536407; Thu, 01 Dec 2016 05:05:36 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.47.92 with HTTP; Thu, 1 Dec 2016 05:05:35 -0800 (PST) In-Reply-To: References: From: Prathamesh Kulkarni Date: Thu, 1 Dec 2016 18:35:35 +0530 Message-ID: Subject: Re: [tree-tailcall] Check if function returns it's argument To: Richard Biener Cc: Jeff Law , gcc Patches X-IsSubscribed: yes On 1 December 2016 at 18:26, Richard Biener wrote: > On Thu, 1 Dec 2016, Prathamesh Kulkarni wrote: > >> On 1 December 2016 at 17:40, Richard Biener wrote: >> > On Thu, 1 Dec 2016, Prathamesh Kulkarni wrote: >> > >> >> On 25 November 2016 at 21:17, Jeff Law wrote: >> >> > On 11/25/2016 01:07 AM, Richard Biener wrote: >> >> > >> >> >>> For the tail-call, issue should we artificially create a lhs and use that >> >> >>> as return value (perhaps by a separate pass before tailcall) ? >> >> >>> >> >> >>> __builtin_memcpy (a1, a2, a3); >> >> >>> return a1; >> >> >>> >> >> >>> gets transformed to: >> >> >>> _1 = __builtin_memcpy (a1, a2, a3) >> >> >>> return _1; >> >> >>> >> >> >>> So tail-call optimization pass would see the IL in it's expected form. >> >> >> >> >> >> >> >> >> As said, a RTL expert needs to chime in here. Iff then tail-call >> >> >> itself should do this rewrite. But if this form is required to make >> >> >> things work (I suppose you checked it _does_ actually work?) then >> >> >> we'd need to make sure later passes do not undo it. So it looks >> >> >> fragile to me. OTOH I seem to remember that the flags we set on >> >> >> GIMPLE are merely a hint to RTL expansion and the tailcalling is >> >> >> verified again there? >> >> > >> >> > So tail calling actually sits on the border between trees and RTL. >> >> > Essentially it's an expand-time decision as we use information from trees as >> >> > well as low level target information. >> >> > >> >> > I would not expect the former sequence to tail call. The tail calling code >> >> > does not know that the return value from memcpy will be a1. Thus the tail >> >> > calling code has to assume that it'll have to copy a1 into the return >> >> > register after returning from memcpy, which obviously can't be done if we >> >> > tail called memcpy. >> >> > >> >> > The second form is much more likely to turn into a tail call sequence >> >> > because the return value from memcpy will be sitting in the proper register. >> >> > This form out to work for most calling conventions that allow tail calls. >> >> > >> >> > We could (in theory) try and exploit the fact that memcpy returns its first >> >> > argument as a return value, but that would only be helpful on a target where >> >> > the first argument and return value use the same register. So I'd have a >> >> > slight preference to rewriting per Prathamesh's suggestion above since it's >> >> > more general. >> >> Thanks for the suggestion. The attached patch creates artificial lhs, >> >> and returns it if the function returns it's argument and that argument >> >> is used as return-value. >> >> >> >> eg: >> >> f (void * a1, void * a2, long unsigned int a3) >> >> { >> >> [0.0%]: >> >> # .MEM_5 = VDEF <.MEM_1(D)> >> >> __builtin_memcpy (a1_2(D), a2_3(D), a3_4(D)); >> >> # VUSE <.MEM_5> >> >> return a1_2(D); >> >> >> >> } >> >> >> >> is transformed to: >> >> f (void * a1, void * a2, long unsigned int a3) >> >> { >> >> void * _6; >> >> >> >> [0.0%]: >> >> # .MEM_5 = VDEF <.MEM_1(D)> >> >> _6 = __builtin_memcpy (a1_2(D), a2_3(D), a3_4(D)); >> >> # VUSE <.MEM_5> >> >> return _6; >> >> >> >> } >> >> >> >> While testing, I came across an issue with function f() defined >> >> intail-padding1.C: >> >> struct X >> >> { >> >> ~X() {} >> >> int n; >> >> char d; >> >> }; >> >> >> >> X f() >> >> { >> >> X nrvo; >> >> __builtin_memset (&nrvo, 0, sizeof(X)); >> >> return nrvo; >> >> } >> >> >> >> input to the pass: >> >> X f() () >> >> { >> >> [0.0%]: >> >> # .MEM_3 = VDEF <.MEM_1(D)> >> >> __builtin_memset (nrvo_2(D), 0, 8); >> >> # VUSE <.MEM_3> >> >> return nrvo_2(D); >> >> >> >> } >> >> >> >> verify_gimple_return failed with: >> >> tail-padding1.C:13:1: error: invalid conversion in return statement >> >> } >> >> ^ >> >> struct X >> >> >> >> struct X & >> >> >> >> # VUSE <.MEM_3> >> >> return _4; >> >> >> >> It seems the return type of function (struct X) differs with the type >> >> of return value (struct X&). >> >> Not sure how this is possible ? >> > >> > You need to honor DECL_BY_REFERENCE of DECL_RESULT. >> Thanks! Gating on !DECL_BY_REFERENCE (DECL_RESULT (cfun->decl)) >> resolved the error. >> Does the attached version look OK ? > > + ass_var = make_ssa_name (TREE_TYPE (arg)); > > can you try > > ass_var = copy_ssa_name (arg); > > instead? That way the underlying decl should make sure the > DECL_BY_REFERENCE check in the IL verification works. Done in the attached version and verified tail-padding1.C passes with the change. Does it look OK ? Bootstrap+test in progress on x86_64-unknown-linux-gnu. Thanks, Prathamesh > > Thanks, > Richard. > > >> Validation in progress. >> >> Thanks, >> Prathamesh >> > >> >> To work around that, I guarded the transform on: >> >> useless_type_conversion_p (TREE_TYPE (TREE_TYPE (cfun->decl)), >> >> TREE_TYPE (retval))) >> >> >> >> in the patch. Does that look OK ? >> >> >> >> Bootstrap+tested on x86_64-unknown-linux-gnu with --enable-languages=all,ada. >> >> Cross-tested on arm*-*-*, aarch64*-*-*. >> >> >> >> Thanks, >> >> Prathamesh >> >> > >> >> > >> >> > Jeff >> >> >> > >> > -- >> > Richard Biener >> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) >> > > -- > Richard Biener > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-9.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-9.c new file mode 100644 index 0000000..b3fdc6c --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-9.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-tailc-details" } */ + +void *f(void *a1, void *a2, __SIZE_TYPE__ a3) +{ + __builtin_memcpy (a1, a2, a3); + return a1; +} + +/* { dg-final { scan-tree-dump-times "Found tail call" 1 "tailc" } } */ diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c index 66a0a4c..6135dc2 100644 --- a/gcc/tree-tailcall.c +++ b/gcc/tree-tailcall.c @@ -401,6 +401,7 @@ find_tail_calls (basic_block bb, struct tailcall **ret) basic_block abb; size_t idx; tree var; + greturn *ret_stmt = NULL; if (!single_succ_p (bb)) return; @@ -408,6 +409,8 @@ find_tail_calls (basic_block bb, struct tailcall **ret) for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi)) { stmt = gsi_stmt (gsi); + if (!ret_stmt) + ret_stmt = dyn_cast (stmt); /* Ignore labels, returns, nops, clobbers and debug stmts. */ if (gimple_code (stmt) == GIMPLE_LABEL @@ -422,6 +425,36 @@ find_tail_calls (basic_block bb, struct tailcall **ret) { call = as_a (stmt); ass_var = gimple_call_lhs (call); + if (!ass_var) + { + /* Check if function returns one if it's arguments + and that argument is used as return value. + In that case create an artificial lhs to call_stmt, + and set it as the return value. */ + + unsigned rf = gimple_call_return_flags (call); + if (rf & ERF_RETURNS_ARG) + { + unsigned argnum = rf & ERF_RETURN_ARG_MASK; + if (argnum < gimple_call_num_args (call) + && ret_stmt) + { + tree arg = gimple_call_arg (call, argnum); + tree retval = gimple_return_retval (ret_stmt); + if (retval + && TREE_CODE (retval) == SSA_NAME + && operand_equal_p (retval, arg, 0) + && !DECL_BY_REFERENCE (DECL_RESULT (cfun->decl))) + { + ass_var = copy_ssa_name (arg); + gimple_call_set_lhs (call, ass_var); + update_stmt (call); + gimple_return_set_retval (ret_stmt, ass_var); + update_stmt (ret_stmt); + } + } + } + } break; }