From patchwork Fri Dec 2 06:03:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Prathamesh Kulkarni X-Patchwork-Id: 86240 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp114168qgi; Thu, 1 Dec 2016 22:04:17 -0800 (PST) X-Received: by 10.99.237.17 with SMTP id d17mr75553388pgi.48.1480658657362; Thu, 01 Dec 2016 22:04:17 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id m26si3698658pfg.240.2016.12.01.22.04.17 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 01 Dec 2016 22:04:17 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-443293-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-443293-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-443293-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=sV/NoT1kNzS4YGO jAhyIbbz1EQ889cPleq3/zn73iI0Qfm+zMFlGOOyIBvodBMzhBYlceeyoDMwIhKU w2IE+sE34/k1rnbW+1RQJaT3dvPimny6DCKF85R8oC/rDgh+6IY5map+dW08DjX4 qT/HsccBhcYZ4PrM+t/XaWM+YLmw= 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=fXejk8HVUBhbmsbsyhmFv X6hVOg=; b=OvB5O5raZQ9JAP3x8VrLc/1MiwKWUVU99yM9frk/StyU/kvG5/+Z2 sstJjWV0yo/kr9A0hIvQIrkTxmrPVaW1/FqXl4F5bVVsBfzxsoZts1QNHPKPzN2w aiC2x6pBJdanrH47u+82mx3guFJ1guGptE7x6aV/zs3kQfWs1L3Fsc= Received: (qmail 100036 invoked by alias); 2 Dec 2016 06:04:00 -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 98011 invoked by uid 89); 2 Dec 2016 06:03:58 -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, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=no version=3.3.2 spammy=abb, 4098, richards, Richards X-HELO: mail-io0-f176.google.com Received: from mail-io0-f176.google.com (HELO mail-io0-f176.google.com) (209.85.223.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Dec 2016 06:03:48 +0000 Received: by mail-io0-f176.google.com with SMTP id a124so465186563ioe.2 for ; Thu, 01 Dec 2016 22:03:48 -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=Jnyknxocz5HpBYyBF5ZtI4p3/kCxOrY0UgGlpP+60OE=; b=CAW02GU5J6I3sCCovrSbci2RY+29HAXn7jM0DZi7IpK8PJSxrQaWPAMhNJRLDFGmlL pSRwDOM+NzHZBdSORE8/52HpTKFRsV0dROJ8LtwVM/9jorAjJdJ9yip9ZShZtPP4n6r2 Ndlq414zYK6C1pDKA8HwnL9qfULq7xXIOqc8A5QyJP5n3uPvym5orPkeKDW1BaGdqr09 McD73cp56tk0jBgLUhM02QjJ50leEoEZbSqTbiAy1yMcOiV9+8iaO8B3yL1MZDo3gXbI ZM94sLOT/TWmnESSns3YZ3V0hTggSZF7NkTsMi1v7dPcoM5DEKeLEzHMbSNzZPhEfVmQ y93w== X-Gm-Message-State: AKaTC01Ploo8512Yor1E+TKueuxr7sPvOXsIxoaD1/N6iT+/Va4otgPtKrhgSqGJb4dztNmZC5lSKruvoosJU7Jp X-Received: by 10.36.43.193 with SMTP id h184mr1242101ita.29.1480658626607; Thu, 01 Dec 2016 22:03:46 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.47.92 with HTTP; Thu, 1 Dec 2016 22:03:45 -0800 (PST) In-Reply-To: <8a57d3a7-6885-359f-be1e-c72b7802c5b2@redhat.com> References: <8a57d3a7-6885-359f-be1e-c72b7802c5b2@redhat.com> From: Prathamesh Kulkarni Date: Fri, 2 Dec 2016 11:33:45 +0530 Message-ID: Subject: Re: [tree-tailcall] Check if function returns it's argument To: Jeff Law Cc: Richard Biener , gcc Patches X-IsSubscribed: yes On 2 December 2016 at 03:57, Jeff Law wrote: > On 12/01/2016 06:22 AM, Richard Biener wrote: >>> >>> Well after removing DECL_BY_REFERENCE, verify_gimple still fails but >>> differently: >>> >>> tail-padding1.C:13:1: error: RESULT_DECL should be read only when >>> DECL_BY_REFERENCE is set >>> } >>> ^ >>> while verifying SSA_NAME nrvo_4 in statement >>> # .MEM_3 = VDEF <.MEM_1(D)> >>> nrvo_4 = __builtin_memset (nrvo_2(D), 0, 8); >>> tail-padding1.C:13:1: internal compiler error: verify_ssa failed >> >> >> Hmm, ok. Not sure why we enforce this. > > I don't know either. But I would start by looking at tree-nrv.c since it > looks (based on the variable names) that the named-value-return optimization > kicked in. Um, the name nrv0 was in the test-case itself. The transform takes place in tailr1 pass, which appears to be before nrv, so possibly this is not related to nrv ? The verify check seems to be added in r161898 by Honza to fix PR 44813 based on Richard's following suggestion from https://gcc.gnu.org/ml/gcc-patches/2010-07/msg00358.html: "We should never see a defintion of a RESULT_DECL SSA name for DECL_BY_REFERENCE RESULT_DECLs (that would be a bug - we should add verification to the SSA verifier, can you do add that?)." The attached patch moves && ret_stmt together with !ass_var, and keeps the !DECL_BY_REFERENCE (DECL_RESULT (cfun->decl)) check, and adjusts tailcall-9.c testcase to scan _\[0-9\]* = __builtin_memcpy in tailr1 dump since that's where the transform takes place. Is this version OK ? Thanks, Prathamesh > >> >> Note that in the end this patch looks fishy -- iff we really need >> the LHS on the assignment for correctness if we have the tailcall >> flag set then what guarantees that later passes do not remove it >> again? So anybody removing a LHS would need to unset the tailcall flag? >> >> Saying again that I don't know enough about the RTL part of tailcall >> expansion. > > The LHS on the assignment makes it easier to identify when a tail call is > possible. It's not needed for correctness. Not having the LHS on the > assignment just means we won't get an optimized tail call. > > Under what circumstances would the LHS possibly be removed? We know the > return statement references the LHS, so it's not going to be something that > DCE will do. > > jeff 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..9c482f4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-9.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-tailr-details" } */ + +void *f(void *a1, void *a2, __SIZE_TYPE__ a3) +{ + __builtin_memcpy (a1, a2, a3); + return a1; +} + +/* { dg-final { scan-tree-dump "_\[0-9\]* = __builtin_memcpy" "tailr1" } } */ diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c index 66a0a4c..64f624f 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,35 @@ find_tail_calls (basic_block bb, struct tailcall **ret) { call = as_a (stmt); ass_var = gimple_call_lhs (call); + if (!ass_var && ret_stmt) + { + /* 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)) + { + 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; }