From patchwork Thu Dec 1 12:34:45 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Diego Novillo X-Patchwork-Id: 128696 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]) by ozlabs.org (Postfix) with SMTP id E6BB1B6F75 for ; Thu, 1 Dec 2011 23:35:09 +1100 (EST) Received: (qmail 326 invoked by alias); 1 Dec 2011 12:35:03 -0000 Received: (qmail 32653 invoked by uid 22791); 1 Dec 2011 12:35:01 -0000 X-SWARE-Spam-Status: No, hits=-3.0 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, TW_SV, TW_TM X-Spam-Check-By: sourceware.org Received: from mail-ee0-f47.google.com (HELO mail-ee0-f47.google.com) (74.125.83.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 01 Dec 2011 12:34:47 +0000 Received: by eekc1 with SMTP id c1so1242465eek.20 for ; Thu, 01 Dec 2011 04:34:46 -0800 (PST) Received: by 10.216.24.21 with SMTP id w21mr364541wew.57.1322742886013; Thu, 01 Dec 2011 04:34:46 -0800 (PST) MIME-Version: 1.0 Received: by 10.216.24.21 with SMTP id w21mr364529wew.57.1322742885773; Thu, 01 Dec 2011 04:34:45 -0800 (PST) Received: by 10.227.35.195 with HTTP; Thu, 1 Dec 2011 04:34:45 -0800 (PST) In-Reply-To: References: Date: Thu, 1 Dec 2011 07:34:45 -0500 Message-ID: Subject: Re: [PATCH] Fix early inliner inlining uninlinable functions From: Diego Novillo To: Richard Guenther Cc: "H.J. Lu" , Iain Sandoe , gcc-patches Patches , Jan Hubicka , Eric Botcazou X-System-Of-Record: true X-IsSubscribed: yes 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 On Thu, Dec 1, 2011 at 07:08, Richard Guenther wrote: > Sure, but then you can still have the issue of an inconsistency. Not if we make the edge attribute secondary to the statement attribute. Given that can_inline_edge_p() is the *only* tester for this attribute, what I was thinking was to change can_inline_edge_p() to: if (!inlinable && report) report_inline_failed_reason (e); return inlinable; > Thus, would you then remove the remaining asserts? The asserts disappear because we have weakened the meaning of the edge attribute. It is only usable when there is no statement on it. The question now is, how do we know that the attribute is not lying? This only happens in WPA mode, so it would then become an issue of pessimization, not correctness. > I believe in the end the proper fix is to _not_ throw away > cgraph edges all the time, but keep them up-to-date and thus > make the stmt flag not necessary. Make it a pure cgraph attribute? Sure, anything that gets rid of the dual attribute is the way to go. There are not very many invocations to the gimple attribute, but I don't know how big a change that is. > Which pass did the folding of the stmt but did not adjust the > edge flag? The new call to gimple_call_set_cannot_inline added by this patch: commit 3aa6ac67f5f7d3a6aabce9ada30e99e2a82c0114 Author: rguenth Date: Wed Nov 2 08:46:08 2011 +0000 2010-11-02 Richard Guenther PR tree-optimization/50890 * gimple.h (gimple_fold_call): Remove. * gimple-fold.c (fold_stmt_1): Move all call related code to ... (gimple_fold_call): ... here. Make static. Update the cannot-inline flag on direct calls. * ipa-inline.c (early_inliner): Copy the cannot-inline flag from the statements to the edges. * gcc.dg/torture/pr50890.c: New testcase. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@180763 138bc75d-0d04-0410-961f-82ee72b054a4 Diego. diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c index 3dadf8d..e3c6b3c 100644 --- a/gcc/ipa-inline.c +++ b/gcc/ipa-inline.c @@ -246,6 +246,14 @@ can_inline_edge_p (struct cgraph_edge *e, bool report) struct function *caller_cfun = DECL_STRUCT_FUNCTION (e->caller->decl); struct function *callee_cfun = callee ? DECL_STRUCT_FUNCTION (callee->decl) : NULL; + bool call_stmt_cannot_inline_p; + + /* If E has a call statement in it, use the inline attribute from + the statement, otherwise use the inline attribute in E. Edges + will not have statements when working in WPA mode. */ + call_stmt_cannot_inline_p = (e->call_stmt) + ? gimple_call_cannot_inline_p (e->call_stmt) + : e->call_stmt_cannot_inline_p; if (!caller_cfun && e->caller->clone_of) caller_cfun = DECL_STRUCT_FUNCTION (e->caller->clone_of->decl); @@ -270,7 +278,7 @@ can_inline_edge_p (struct cgraph_edge *e, bool report) e->inline_failed = CIF_OVERWRITABLE; return false; } - else if (e->call_stmt_cannot_inline_p) + else if (call_stmt_cannot_inline_p) { e->inline_failed = CIF_MISMATCHED_ARGUMENTS; inlinable = false; @@ -343,14 +351,6 @@ can_inline_edge_p (struct cgraph_edge *e, bool report) } } - /* Be sure that the cannot_inline_p flag is up to date. */ - gcc_checking_assert (!e->call_stmt - || (gimple_call_cannot_inline_p (e->call_stmt) - == e->call_stmt_cannot_inline_p) - /* In -flto-partition=none mode we really keep things out of - sync because call_stmt_cannot_inline_p is set at cgraph - merging when function bodies are not there yet. */ - || (in_lto_p && !gimple_call_cannot_inline_p (e->call_stmt)));