From patchwork Fri Mar 10 15:07:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 737436 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 3vfrGZ1gzNz9s5g for ; Sat, 11 Mar 2017 02:07:33 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="MiO+Ptw7"; 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:date :from:to:cc:subject:message-id:mime-version:content-type; q=dns; s=default; b=As+GdAeLG473BxImorWssRDdIcPoyFic1+WQRn3OMUwmah+AoC Gk3ySpakOTNy23JAlxurtP2xD1WN17nsmI3tj6eb2lhK6sK26swYOag8z3yt4E4P pRsKssMzyWmUqriXcXuaW458Dn9QvHLwx2BzkTqbKAsawdInxa9yFqfx0= 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:date :from:to:cc:subject:message-id:mime-version:content-type; s= default; bh=lyujpIfzkYe+SkgsKCRyUEWtv8s=; b=MiO+Ptw74/0WkrFjFMVS bbV9SUWyPOFoaWsFHByuXEufI1AmIyxnL4MaTZG4AxZwwHk5jxPBlHeVUUR5+S22 LK4Z41djgjN1A4F+6BCNbDbqMcHz23chJtrcbhD2Tv05Ji5bto8mInY4CNnTnugq jDdzjbW7GeV0sBsvIm+C/SI= Received: (qmail 102857 invoked by alias); 10 Mar 2017 15:07:23 -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 102841 invoked by uid 89); 10 Mar 2017 15:07:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy=recheck, hypothetical X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Mar 2017 15:07:20 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A803BABC9 for ; Fri, 10 Mar 2017 15:07:18 +0000 (UTC) Date: Fri, 10 Mar 2017 16:07:18 +0100 From: Martin Jambor To: GCC Patches Cc: Jan Hubicka , Richard Biener Subject: [PR 77333] Fix fntypes of calls calling clones Message-ID: <20170310150718.xeg374b2z3lgicon@virgil.suse.cz> Mail-Followup-To: GCC Patches , Jan Hubicka , Richard Biener MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.2 (2016-07-01) X-IsSubscribed: yes Hi, PR 77333 is a i686-windows target bug, which however has its root in our general mechanism of adjusting gimple statements when redirecting call graph edge. Basically, these three things trigger it: 1) IPA-CP figures out that the this parameter of a C++ class method is unused and because the class is in an anonymous namespace, it can be removed and all calls adjusted. That effectively changes a normal method into a static method and so internally, its type changes from METHOD_TYPE to FUNCTION_TYPE. 2) Since the fix of PR 57330, we do not update gimple_call_fntype to match the new type, in fact we explicitely set it to the old, now invalid, type (see redirect_call_stmt_to_callee in cgraph.c). 3) Function ix86_get_callcvt which decides on call ABI, ends with the following condition: if (ret != 0 || is_stdarg || TREE_CODE (type) != METHOD_TYPE || ix86_function_type_abi (type) != MS_ABI) return IX86_CALLCVT_CDECL | ret; return IX86_CALLCVT_THISCALL; ...and since now the callee is no longer a METHOD_TYPE but callers still think that they are, leading to calling convention mismatches and subsequent crashes. It took me quite a lot of time to come up with a small testcase (reproducible using wine) but eventually I managed. The fix is not to do 2) above, but doing so without re-introducing PR 57330, of course. There are two options. One is to use the call_stmt_cannot_inline_p flag of call-graph edges and not do any IPA-CP accross those edges. That is done in the patch below. The (so far a bit hypothetical) problem with that approach is that the call graph edge flag may not be 100% reliable in LTO, because incompatible decls might get merged and then we wold re-introduce PR 57330 again - only with on invalid code and with LTO but an ICE nevertheless. So the alternative would be to re-check when doing the gimple statement adjustment and if the types match, then set the correct new gimple_fntype and if they don't... then we can either leave it be or just run the same type transformation on it as we did on the callee, though they would be bogus either way. That is implemented in the attached patch. I have successfully bootstrapped both patches on x86_64-linux and I have also tested them both on a windows cross-compiler and wine (with some noise but I believe it is just noise). Honza, Richi, do you prefer any one approach over the other? Actually, we can have both, would that be desirable? Thanks, Martin 2017-03-02 Martin Jambor PR ipa/77333 * ipa-prop.h (ipa_node_params): New field call_stmt_type_mismatch. (ipa_node_params::ipa_node_params): Initialize it to zero. * cgraph.c (redirect_call_stmt_to_callee): Set gimple fntype to the type of the new target. * ipa-cp.c (propagate_constants_across_call): Set variable flag of lattices and call_stmt_type_mismatch of the callee when encountering an edge with mismatched types. (estimate_local_effects): Do not clone for all contexts when call_stmt_type_mismatch is set. testsuite/ * g++.dg/ipa/pr77333.C: New test. --- gcc/cgraph.c | 2 +- gcc/ipa-cp.c | 11 ++++--- gcc/ipa-prop.h | 4 ++- gcc/testsuite/g++.dg/ipa/pr77333.C | 65 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr77333.C [PR 77333] Fixup fntypes of gimple calls of clones 2017-03-02 Martin Jambor PR ipa/77333 * cgraph.h (cgraph_build_function_type_skip_args): Declare. * cgraph.c (redirect_call_stmt_to_callee): Set gimple fntype so that it reflects the signature changes performed at the callee side. * cgraphclones.c (build_function_type_skip_args): Make public, renamed to cgraph_build_function_type_skip_args. (build_function_decl_skip_args): Adjust call to the above function. testsuite/ * g++.dg/ipa/pr77333.C: New test. --- gcc/cgraph.c | 16 +++++++++- gcc/cgraph.h | 2 ++ gcc/cgraphclones.c | 9 +++--- gcc/testsuite/g++.dg/ipa/pr77333.C | 65 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr77333.C diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 839388496ee..5143eafa2a6 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1424,8 +1424,22 @@ cgraph_edge::redirect_call_stmt_to_callee (void) if (skip_bounds) new_stmt = chkp_copy_call_skip_bounds (new_stmt); + tree old_fntype = gimple_call_fntype (e->call_stmt); + tree old_fndecl = gimple_call_fndecl (e->call_stmt); gimple_call_set_fndecl (new_stmt, e->callee->decl); - gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt)); + if ((e->callee->former_clone_of + && old_fntype == TREE_TYPE (e->callee->former_clone_of)) + || (old_fndecl + && gimple_check_call_matching_types (e->call_stmt, + old_fndecl, false))) + gimple_call_set_fntype (new_stmt, TREE_TYPE (e->callee->decl)); + else + { + bitmap skip = e->callee->clone.combined_args_to_skip; + tree t = cgraph_build_function_type_skip_args (old_fntype, skip, + false); + gimple_call_set_fntype (new_stmt, t); + } if (gimple_vdef (new_stmt) && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME) diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 3889a3e1701..62cebd9e55a 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -2326,6 +2326,8 @@ void tree_function_versioning (tree, tree, vec *, void dump_callgraph_transformation (const cgraph_node *original, const cgraph_node *clone, const char *suffix); +tree cgraph_build_function_type_skip_args (tree orig_type, bitmap args_to_skip, + bool skip_return); /* In cgraphbuild.c */ int compute_call_stmt_bb_frequency (tree, basic_block bb); diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index c2337e84553..69572b926c4 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -152,9 +152,9 @@ cgraph_edge::clone (cgraph_node *n, gcall *call_stmt, unsigned stmt_uid, /* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the return value if SKIP_RETURN is true. */ -static tree -build_function_type_skip_args (tree orig_type, bitmap args_to_skip, - bool skip_return) +tree +cgraph_build_function_type_skip_args (tree orig_type, bitmap args_to_skip, + bool skip_return) { tree new_type = NULL; tree args, new_args = NULL; @@ -219,7 +219,8 @@ build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip, if (prototype_p (new_type) || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type)))) new_type - = build_function_type_skip_args (new_type, args_to_skip, skip_return); + = cgraph_build_function_type_skip_args (new_type, args_to_skip, + skip_return); TREE_TYPE (new_decl) = new_type; /* For declarations setting DECL_VINDEX (i.e. methods) diff --git a/gcc/testsuite/g++.dg/ipa/pr77333.C b/gcc/testsuite/g++.dg/ipa/pr77333.C new file mode 100644 index 00000000000..1ef997f7a54 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr77333.C @@ -0,0 +1,65 @@ +// { dg-do run } +// { dg-options "-O2 -fno-ipa-sra" } + +volatile int global; +int __attribute__((noinline, noclone)) +get_data (int i) +{ + global = i; + return i; +} + +typedef int array[32]; + +namespace { + +char buf[512]; + +class A +{ +public: + int field; + char *s; + + A() : field(223344) + { + s = buf; + } + + int __attribute__((noinline)) + foo (int a, int b, int c, int d, int e, int f, int g, int h, int i, int j, + int k, int l, int m, int n, int o, int p, int q, int r, int s, int t) + { + global = a+b+c+d+e+f+g+h+i+j+k+l+m+n+o+p+q+r+s+t; + return global; + } + + int __attribute__((noinline)) + bar() + { + int r = foo (get_data (1), get_data (1), get_data (1), get_data (1), + get_data (1), get_data (1), get_data (1), get_data (1), + get_data (1), get_data (1), get_data (1), get_data (1), + get_data (1), get_data (1), get_data (1), get_data (1), + get_data (1), get_data (1), get_data (1), get_data (1)); + + if (field != 223344) + __builtin_abort (); + return 0; + } +}; + +} + +int main (int argc, char **argv) +{ + A a; + int r = a.bar(); + r = a.bar (); + if (a.field != 223344) + __builtin_abort (); + if (global != 20) + __builtin_abort (); + + return r; +} -- 2.12.0