From patchwork Fri Mar 8 13:46:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 86974 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B0B1C385DC1D for ; Fri, 8 Mar 2024 14:26:19 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2a07:de40:b251:101:10:150:64:1]) by sourceware.org (Postfix) with ESMTPS id 07107385DC18 for ; Fri, 8 Mar 2024 14:25:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 07107385DC18 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 07107385DC18 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a07:de40:b251:101:10:150:64:1 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709907945; cv=none; b=Ja2mIuLoE4JKfKKjWz8x85TyWOJvLNDB/GuI9150fVrFEZSVaQJCdT7ekDpx3G2jIN2EsOhsY6lbjswwfyOpMBpyj/YtrnSyrKQbl/Ho84bZAadEDOD4vj2CxeQbQPbONjzzJTqUBnDNORCWQ2c6evcIPiTaeX/sZzuyfE/cm6k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709907945; c=relaxed/simple; bh=I4pLij1tMlmphRWZI9PIEfkXbGWn+zlhYDi8dlNjbS4=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:From: To:Subject:Date:Message-ID:MIME-Version; b=lyBYdDtzgr/zGPuPFcgm+oeWx89FxeYOw2vprp7prPC9ta2AvtPNztDIFNZS1PMUKV/I+g+eOSte/C785y84vPGWItqu/Gq5aj9mXuYHcuTgOgtJ12EDfcb40dxVPJG/m4vAbmWSrnTH4qXEppqQbZ+bsuVZgBoonYFWrXHP+0U= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 2FD76353BF; Fri, 8 Mar 2024 13:46:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1709905604; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=Te3dul4fT/7tLA7OP74pYGYW9R5m59IlXcUTav8I+xI=; b=em5V0eVU6ThSvrWnPhbL0QLqHJyqPBKPYsd6c1D6FaIxcpHRBraradHyfoSSVWbIPga9Y5 pHgcny94bXPe/oWcL6T167Y094R56m/jrKrtIWREaAM/Sh0FfCfiW6TiJqeVJE7JQE9IDD W6KOSy/T3TPBA4Z5JT+SxJJY2DFbJOU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1709905604; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=Te3dul4fT/7tLA7OP74pYGYW9R5m59IlXcUTav8I+xI=; b=3NNsI8SinuSWey1ygmi9KGB54d7CC3hYbNEQ7vz+sKQG//mjBHhGOcIbw3Lm1UM3oq/RMM q1t0/xu2dwC26UBg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1709905604; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=Te3dul4fT/7tLA7OP74pYGYW9R5m59IlXcUTav8I+xI=; b=em5V0eVU6ThSvrWnPhbL0QLqHJyqPBKPYsd6c1D6FaIxcpHRBraradHyfoSSVWbIPga9Y5 pHgcny94bXPe/oWcL6T167Y094R56m/jrKrtIWREaAM/Sh0FfCfiW6TiJqeVJE7JQE9IDD W6KOSy/T3TPBA4Z5JT+SxJJY2DFbJOU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1709905604; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=Te3dul4fT/7tLA7OP74pYGYW9R5m59IlXcUTav8I+xI=; b=3NNsI8SinuSWey1ygmi9KGB54d7CC3hYbNEQ7vz+sKQG//mjBHhGOcIbw3Lm1UM3oq/RMM q1t0/xu2dwC26UBg== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 2A07A13310; Fri, 8 Mar 2024 13:46:44 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id P2Q5CsQW62UkIwAAD6G6ig (envelope-from ); Fri, 08 Mar 2024 13:46:44 +0000 From: Martin Jambor To: GCC Patches Cc: Jan Hubicka Subject: [PATCH] ipa: Fix C++ member ptr indirect inlining (PR 114254, PR 108802) User-Agent: Notmuch/0.38.2 (https://notmuchmail.org) Emacs/29.2 (x86_64-suse-linux-gnu) Date: Fri, 08 Mar 2024 14:46:43 +0100 Message-ID: MIME-Version: 1.0 X-Spam-Level: Authentication-Results: smtp-out1.suse.de; none X-Spamd-Result: default: False [-3.10 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; URIBL_BLOCKED(0.00)[suse.cz:email,ipa-prop.cc:url]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; RCVD_COUNT_THREE(0.00)[3]; MID_RHS_MATCH_FROMTLD(0.00)[]; DKIM_SIGNED(0.00)[suse.cz:s=susede2_rsa,suse.cz:s=susede2_ed25519]; TO_DN_ALL(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.cz:email]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] X-Spam-Score: -3.10 X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Hi, Even though we have had code to handle creation of indirect call graph edges (so that these calls can than be made direct as part of IPA-CP and inlining and eventually also inlined) for C++ member pointers for many years, it turns out that it does not work for lambdas and that it has been severely broken since GCC 10 when the base class has virtual functions. Lambdas don't work because the code cannot work with structures representing member function pointers because they are passed by reference instead by value and the code was not ready for that. The presence of virtual methods broke thinks because at some point C++ FE got clever and stopped emitting the check for virtual methods when the base class does not have any and that in turn made our existing testcases not test the necessary pattern matching code. The pattern matcher had a small bug which did not matter before r10-917-g3b47da42de621c but did afterwards. This patch changes the pattern matcher to match both of these cases. Special thanks to the Linaro automated checker of patches which reported that the earlier version of my PR 108802 fix was not working on Aarch64 which in turn made me discover PR 114254. The patch has passed bootstrap and testing on x86_64-linux, aarch64-linux and ppc64-linux and I also LTO bootstrap on x86_64-linux. I understand we have been living with these deficiencies for a while now but both are technically regressions. If Honza agrees (and manages to review the patch quickly), I'm fine with pushing them to master now but I can also wait until the next stage 1. Thanks, Martin gcc/ChangeLog: 2024-03-06 Martin Jambor PR ipa/108802 PR ipa/114254 * ipa-prop.cc (ipa_get_stmt_member_ptr_load_param): Fix case looking at COMPONENT_REFs directly from a PARM_DECL, also recognize loads from a pointer parameter. (ipa_analyze_indirect_call_uses): Also recognize loads from a pointer parameter, also recognize the case when pfn pointer is loaded in its own BB. gcc/testsuite/ChangeLog: 2024-03-06 Martin Jambor PR ipa/108802 PR ipa/114254 * g++.dg/ipa/iinline-4.C: New test. * g++.dg/ipa/pr108802.C: Likewise. --- gcc/ipa-prop.cc | 110 +++++++++++++++++++-------- gcc/testsuite/g++.dg/ipa/iinline-4.C | 61 +++++++++++++++ gcc/testsuite/g++.dg/ipa/pr108802.C | 14 ++++ 3 files changed, 154 insertions(+), 31 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/iinline-4.C create mode 100644 gcc/testsuite/g++.dg/ipa/pr108802.C diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index e22c4f78405..e8e4918d5a8 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -2500,7 +2500,9 @@ static tree ipa_get_stmt_member_ptr_load_param (gimple *stmt, bool use_delta, HOST_WIDE_INT *offset_p) { - tree rhs, rec, ref_field, ref_offset, fld, ptr_field, delta_field; + tree rhs, fld, ptr_field, delta_field; + tree ref_field = NULL_TREE; + tree ref_offset = NULL_TREE; if (!gimple_assign_single_p (stmt)) return NULL_TREE; @@ -2511,35 +2513,53 @@ ipa_get_stmt_member_ptr_load_param (gimple *stmt, bool use_delta, ref_field = TREE_OPERAND (rhs, 1); rhs = TREE_OPERAND (rhs, 0); } + + if (TREE_CODE (rhs) == MEM_REF) + { + ref_offset = TREE_OPERAND (rhs, 1); + if (ref_field && integer_nonzerop (ref_offset)) + return NULL_TREE; + } + else if (!ref_field) + return NULL_TREE; + + if (TREE_CODE (rhs) == MEM_REF + && TREE_CODE (TREE_OPERAND (rhs, 0)) == SSA_NAME + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (rhs, 0))) + { + rhs = TREE_OPERAND (rhs, 0); + if (TREE_CODE (SSA_NAME_VAR (rhs)) != PARM_DECL + || !type_like_member_ptr_p (TREE_TYPE (TREE_TYPE (rhs)), &ptr_field, + &delta_field)) + return NULL_TREE; + } else - ref_field = NULL_TREE; - if (TREE_CODE (rhs) != MEM_REF) - return NULL_TREE; - rec = TREE_OPERAND (rhs, 0); - if (TREE_CODE (rec) != ADDR_EXPR) - return NULL_TREE; - rec = TREE_OPERAND (rec, 0); - if (TREE_CODE (rec) != PARM_DECL - || !type_like_member_ptr_p (TREE_TYPE (rec), &ptr_field, &delta_field)) - return NULL_TREE; - ref_offset = TREE_OPERAND (rhs, 1); + { + if (TREE_CODE (rhs) == MEM_REF + && TREE_CODE (TREE_OPERAND (rhs, 0)) == ADDR_EXPR) + rhs = TREE_OPERAND (TREE_OPERAND (rhs, 0), 0); + if (TREE_CODE (rhs) != PARM_DECL + || !type_like_member_ptr_p (TREE_TYPE (rhs), &ptr_field, + &delta_field)) + return NULL_TREE; + } if (use_delta) fld = delta_field; else fld = ptr_field; - if (offset_p) - *offset_p = int_bit_position (fld); if (ref_field) { - if (integer_nonzerop (ref_offset)) + if (ref_field != fld) return NULL_TREE; - return ref_field == fld ? rec : NULL_TREE; } - else - return tree_int_cst_equal (byte_position (fld), ref_offset) ? rec - : NULL_TREE; + else if (!tree_int_cst_equal (byte_position (fld), ref_offset)) + return NULL_TREE; + + if (offset_p) + *offset_p = int_bit_position (fld); + return rhs; } /* Returns true iff T is an SSA_NAME defined by a statement. */ @@ -2585,8 +2605,9 @@ ipa_note_param_call (struct cgraph_node *node, int param_index, describing the call is created. This is very simple for ordinary pointers represented in SSA but not-so-nice when it comes to member pointers. The ugly part of this function does nothing more than trying to match the - pattern of such a call. An example of such a pattern is the gimple dump - below, the call is on the last line: + pattern of such a call. Look up the documentation of macro + TARGET_PTRMEMFUNC_VBIT_LOCATION for details. An example of such a pattern + is the gimple dump below, the call is on the last line: : f$__delta_5 = f.__delta; @@ -2710,10 +2731,23 @@ ipa_analyze_indirect_call_uses (struct ipa_func_body_info *fbi, gcall *call, corresponding to the pattern. */ if (!single_pred_p (virt_bb) || !single_succ_p (virt_bb) - || single_pred (virt_bb) != bb || single_succ (virt_bb) != join) return; + + if (single_pred (virt_bb) != bb) + { + /* In cases when the distinction between a normal and a virtual + function is encoded in the delta field, the load of the + actual non-virtual function pointer can be in its own BB. */ + + if (!single_pred_p (bb) || !single_succ_p (bb)) + return; + bb = single_pred (bb); + if (bb != single_pred (virt_bb)) + return; + } + /* Third, let's see that the branching is done depending on the least significant bit of the pfn. */ @@ -2759,17 +2793,31 @@ ipa_analyze_indirect_call_uses (struct ipa_func_body_info *fbi, gcall *call, if (rec != rec2) return; - index = ipa_get_param_decl_index (info, rec); - if (index >= 0 - && parm_preserved_before_stmt_p (fbi, index, call, rec)) + if (TREE_CODE (rec) == SSA_NAME) { - struct cgraph_edge *cs = ipa_note_param_call (fbi->node, index, - call, false); - cs->indirect_info->offset = offset; - cs->indirect_info->agg_contents = 1; - cs->indirect_info->member_ptr = 1; - cs->indirect_info->guaranteed_unmodified = 1; + index = ipa_get_param_decl_index (info, SSA_NAME_VAR (rec)); + if (index < 0 + || !parm_ref_data_preserved_p (fbi, index, call, + gimple_assign_rhs1 (def))) + return; + by_ref = true; } + else + { + index = ipa_get_param_decl_index (info, rec); + if (index < 0 + || !parm_preserved_before_stmt_p (fbi, index, call, rec)) + return; + by_ref = false; + } + + struct cgraph_edge *cs = ipa_note_param_call (fbi->node, index, + call, false); + cs->indirect_info->offset = offset; + cs->indirect_info->agg_contents = 1; + cs->indirect_info->member_ptr = 1; + cs->indirect_info->by_ref = by_ref; + cs->indirect_info->guaranteed_unmodified = 1; return; } diff --git a/gcc/testsuite/g++.dg/ipa/iinline-4.C b/gcc/testsuite/g++.dg/ipa/iinline-4.C new file mode 100644 index 00000000000..b7ab89bb4d5 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/iinline-4.C @@ -0,0 +1,61 @@ +/* Verify that simple indirect calls are inlined even without early + inlining.. */ +/* { dg-do compile } */ +/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */ +/* { dg-add-options bind_pic_locally } */ + +extern void non_existent (const char *, int); + +class String +{ +private: + const char *data; + +public: + String (const char *d) : data(d) + {} + + int funcOne (int delim) const; + int printStuffTwice (int delim) const; + virtual int whee (int delim) const; +}; + + +int String::funcOne (int delim) const +{ + int i; + for (i = 0; i < delim; i++) + non_existent(data, i); + + return 1; +} + +extern int global; + +int docalling (int c, int (String::* f)(int delim) const) +{ + String S ("muhehehe"); + + if (c > 2) + global = 3; + else + global = 5; + + return (S.*f)(4); +} + +int __attribute__ ((noinline,noclone)) get_input (void) +{ + return 1; +} + +int main (int argc, char *argv[]) +{ + int i = 0; + while (i < 1000) + i += docalling (get_input (), &String::funcOne); + non_existent ("done", i); + return 0; +} + +/* { dg-final { scan-ipa-dump "String::funcOne\[^\\n\]*inline copy in int main" "inline" } } */ diff --git a/gcc/testsuite/g++.dg/ipa/pr108802.C b/gcc/testsuite/g++.dg/ipa/pr108802.C new file mode 100644 index 00000000000..2e2b6c66b64 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr108802.C @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -std=c++14 -fdump-ipa-inline -fno-early-inlining" } */ +/* { dg-add-options bind_pic_locally } */ + +struct A { + int interesting(int x) { return 2 * x; } +}; + +int f1() { + A a; + return [&](auto&& f) { return (a.*f)(42); } (&A::interesting); +} + +/* { dg-final { scan-ipa-dump "A::interesting\[^\\n\]*inline copy in int f1" "inline" } } */