From patchwork Sun Jan 19 15:40:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 1225497 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-517687-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ucw.cz Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=aa3oeDBE; dkim-atps=neutral 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 480zXT0C7nz9sRG for ; Mon, 20 Jan 2020 02:40:38 +1100 (AEDT) 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:subject:message-id:mime-version:content-type; q=dns; s= default; b=f2U4B1LqxBDDaVcVILkokyoeb4fewTp/2a/2qkGoggyZb7C7bffPA le+C2eg5yn0K5CcMxYQbQdDMGH9ZSfVTRplfcb+HMjkZxqsSgT6AvFK4vlPDUScT Lm4Z3BD76TZ6dpHDJoLMSf8c6ebayJkK4A/rOgI76rg5U+Ki9AvFqs= 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:subject:message-id:mime-version:content-type; s= default; bh=mBAX/Dk3WkR3RNABXt7XzsnVxw0=; b=aa3oeDBEP6dJlGfSWSmC vGdKnBpBdTEouYycbvYTirHIjYZMLb2+sB5W9AlQSmOG6bUcM399oPQ0cQM/ZS2+ 7u/3rsUf+ie6w2TwpIvToOTeY3ibenST8WIT0Uwf4CSvWUxytqNdH4xZNyCJ/EZQ VILUj2uBoLStEHhR64gphSI= Received: (qmail 102666 invoked by alias); 19 Jan 2020 15:40:31 -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 102658 invoked by uid 89); 19 Jan 2020 15:40:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy= X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 19 Jan 2020 15:40:28 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id D00EE2805E8; Sun, 19 Jan 2020 16:40:25 +0100 (CET) Date: Sun, 19 Jan 2020 16:40:25 +0100 From: Jan Hubicka To: gcc-patches@gcc.gnu.org Subject: Add speculative call edges verifier and fix one extra bug Message-ID: <20200119154025.GC58691@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) Hi, this patch implements verifier and fixes one bug where speculative calls produced by ipa-devirt ended up having num_speculative_call_targets = 0 instead of 1. lto-profilebootstrapped/regtested x86_64-linux, will commit it shortly. Honza PR lto/93318 * cgraph.c (cgraph_edge::make_speculative): Increase number of speculative targets. (verify_speculative_call): New function (cgraph_node::verify_node): Use it. * ipa-profile.c (ipa_profile): Fix formating; do not set number of speculations. diff --git a/gcc/cgraph.c b/gcc/cgraph.c index c442f334fe2..187f6ed30ba 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1076,6 +1076,7 @@ cgraph_edge::make_speculative (cgraph_node *n2, profile_count direct_count, e2->speculative_id = speculative_id; e2->target_prob = target_prob; e2->in_polymorphic_cdtor = in_polymorphic_cdtor; + indirect_info->num_speculative_call_targets++; count -= e2->count; symtab->call_edge_duplication_hooks (this, e2); ref = n->create_reference (n2, IPA_REF_ADDR, call_stmt); @@ -3148,6 +3149,128 @@ cgraph_edge::verify_corresponds_to_fndecl (tree decl) # pragma GCC diagnostic ignored "-Wformat-diag" #endif +/* Verify consistency of speculative call in NODE corresponding to STMT + and LTO_STMT_UID. If INDIRECT is set, assume that it is the indirect + edge of call sequence. Return true if error is found. + + This function is called to every component of indirect call (direct edges, + indirect edge and refs). To save duplicated work, do full testing only + in that case. */ +static bool +verify_speculative_call (struct cgraph_node *node, gimple *stmt, + unsigned int lto_stmt_uid, + struct cgraph_edge *indirect) +{ + if (indirect == NULL) + { + for (indirect = node->indirect_calls; indirect; + indirect = indirect->next_callee) + if (indirect->call_stmt == stmt + && indirect->lto_stmt_uid == lto_stmt_uid) + break; + if (!indirect) + { + error ("missing indirect call in speculative call sequence"); + return true; + } + if (!indirect->speculative) + { + error ("indirect call in speculative call sequence has no " + "speculative flag"); + return true; + } + return false; + } + + /* Maximal number of targets. We probably will never want to have more than + this. */ + const unsigned int num = 256; + cgraph_edge *direct_calls[num]; + ipa_ref *refs[num]; + + for (unsigned int i = 0; i < num; i++) + { + direct_calls[i] = NULL; + refs[i] = NULL; + } + + for (cgraph_edge *direct = node->callees; direct; + direct = direct->next_callee) + if (direct->call_stmt == stmt && direct->lto_stmt_uid == lto_stmt_uid) + { + if (!direct->speculative) + { + error ("direct call to %s in speculative call sequence has no " + "speculative flag", direct->callee->dump_name ()); + return true; + } + if (direct->speculative_id >= num) + { + error ("direct call to %s in speculative call sequence has " + "speculative_uid %i out of range", + direct->callee->dump_name (), direct->speculative_id); + return true; + } + if (direct_calls[direct->speculative_id]) + { + error ("duplicate direct call to %s in speculative call sequence " + "with speculative_uid %i", + direct->callee->dump_name (), direct->speculative_id); + return true; + } + direct_calls[direct->speculative_id] = direct; + } + + ipa_ref *ref; + for (int i = 0; node->iterate_reference (i, ref); i++) + if (ref->speculative + && ref->stmt == stmt && ref->lto_stmt_uid == lto_stmt_uid) + { + if (ref->speculative_id >= num) + { + error ("direct call to %s in speculative call sequence has " + "speculative_uid %i out of range", + ref->referred->dump_name (), ref->speculative_id); + return true; + } + if (refs[ref->speculative_id]) + { + error ("duplicate reference %s in speculative call sequence " + "with speculative_uid %i", + ref->referred->dump_name (), ref->speculative_id); + return true; + } + refs[ref->speculative_id] = ref; + } + + int num_targets = 0; + for (unsigned int i = 0 ; i < num ; i++) + { + if (refs[i] && !direct_calls[i]) + { + error ("missing direct call for speculation %i", i); + return true; + } + if (!refs[i] && direct_calls[i]) + { + error ("missing ref for speculation %i", i); + return true; + } + if (refs[i] != NULL) + num_targets++; + } + + if (num_targets != indirect->num_speculative_call_targets_p ()) + { + error ("number of speculative targets %i mismatched with " + "num_speculative_targets %i", + num_targets, + indirect->num_speculative_call_targets_p ()); + return true; + } + return false; +} + /* Verify cgraph nodes of given cgraph node. */ DEBUG_FUNCTION void cgraph_node::verify_node (void) @@ -3320,6 +3443,10 @@ cgraph_node::verify_node (void) error ("edge has both cal_stmt and lto_stmt_uid set"); error_found = true; } + if (e->speculative + && verify_speculative_call (e->caller, e->call_stmt, e->lto_stmt_uid, + NULL)) + error_found = true; } for (e = indirect_calls; e; e = e->next_callee) { @@ -3342,7 +3469,24 @@ cgraph_node::verify_node (void) fprintf (stderr, "\n"); error_found = true; } + if (e->speculative + && verify_speculative_call (e->caller, e->call_stmt, e->lto_stmt_uid, + e)) + error_found = true; } + for (i = 0; iterate_reference (i, ref); i++) + { + if (ref->stmt && ref->lto_stmt_uid) + { + error ("reference has both cal_stmt and lto_stmt_uid set"); + error_found = true; + } + if (ref->speculative + && verify_speculative_call (this, ref->stmt, + ref->lto_stmt_uid, NULL)) + error_found = true; + } + if (!callers && inlined_to) { error ("inlined_to pointer is set but no predecessors found"); @@ -3519,13 +3663,6 @@ cgraph_node::verify_node (void) /* No CFG available?! */ gcc_unreachable (); - for (i = 0; iterate_reference (i, ref); i++) - if (ref->stmt && ref->lto_stmt_uid) - { - error ("reference has both cal_stmt and lto_stmt_uid set"); - error_found = true; - } - for (e = callees; e; e = e->next_callee) { if (!e->aux && !e->speculative) diff --git a/gcc/ipa-profile.c b/gcc/ipa-profile.c index 03272f20987..670d9e2fb73 100644 --- a/gcc/ipa-profile.c +++ b/gcc/ipa-profile.c @@ -864,104 +864,104 @@ ipa_profile (void) } unsigned speculative_id = 0; - bool speculative_found = false; for (unsigned i = 0; i < spec_count; i++) - { - speculative_call_target item - = csum->speculative_call_targets[i]; - n2 = find_func_by_profile_id (item.target_id); - if (n2) { - if (dump_file) - { - fprintf (dump_file, "Indirect call -> direct call from" - " other module %s => %s, prob %3.2f\n", - n->dump_name (), - n2->dump_name (), - item.target_probability - / (float) REG_BR_PROB_BASE); - } - if (item.target_probability < REG_BR_PROB_BASE / 2) - { - nuseless++; - if (dump_file) - fprintf (dump_file, - "Not speculating: probability is too low.\n"); - } - else if (!e->maybe_hot_p ()) - { - nuseless++; - if (dump_file) - fprintf (dump_file, - "Not speculating: call is cold.\n"); - } - else if (n2->get_availability () <= AVAIL_INTERPOSABLE - && n2->can_be_discarded_p ()) - { - nuseless++; - if (dump_file) - fprintf (dump_file, - "Not speculating: target is overwritable " - "and can be discarded.\n"); - } - else if (!check_argument_count (n2, e)) + speculative_call_target item + = csum->speculative_call_targets[i]; + n2 = find_func_by_profile_id (item.target_id); + if (n2) { - nmismatch++; if (dump_file) - fprintf (dump_file, - "Not speculating: " - "parameter count mismatch\n"); + { + fprintf (dump_file, + "Indirect call -> direct call from" + " other module %s => %s, prob %3.2f\n", + n->dump_name (), + n2->dump_name (), + item.target_probability + / (float) REG_BR_PROB_BASE); + } + if (item.target_probability < REG_BR_PROB_BASE / 2) + { + nuseless++; + if (dump_file) + fprintf (dump_file, + "Not speculating: " + "probability is too low.\n"); + } + else if (!e->maybe_hot_p ()) + { + nuseless++; + if (dump_file) + fprintf (dump_file, + "Not speculating: call is cold.\n"); + } + else if (n2->get_availability () <= AVAIL_INTERPOSABLE + && n2->can_be_discarded_p ()) + { + nuseless++; + if (dump_file) + fprintf (dump_file, + "Not speculating: target is overwritable " + "and can be discarded.\n"); + } + else if (!check_argument_count (n2, e)) + { + nmismatch++; + if (dump_file) + fprintf (dump_file, + "Not speculating: " + "parameter count mismatch\n"); + } + else if (e->indirect_info->polymorphic + && !opt_for_fn (n->decl, flag_devirtualize) + && !possible_polymorphic_call_target_p (e, n2)) + { + nimpossible++; + if (dump_file) + fprintf (dump_file, + "Not speculating: " + "function is not in the polymorphic " + "call target list\n"); + } + else + { + /* Target may be overwritable, but profile says that + control flow goes to this particular implementation + of N2. Speculate on the local alias to allow + inlining. */ + if (!n2->can_be_discarded_p ()) + { + cgraph_node *alias; + alias = dyn_cast + (n2->noninterposable_alias ()); + if (alias) + n2 = alias; + } + nconverted++; + e->make_speculative (n2, + e->count.apply_probability ( + item.target_probability), + speculative_id, + item.target_probability); + update = true; + speculative_id++; + } } - else if (e->indirect_info->polymorphic - && !opt_for_fn (n->decl, flag_devirtualize) - && !possible_polymorphic_call_target_p (e, n2)) + else { - nimpossible++; if (dump_file) fprintf (dump_file, - "Not speculating: " - "function is not in the polymorphic " - "call target list\n"); - } - else - { - /* Target may be overwritable, but profile says that - control flow goes to this particular implementation - of N2. Speculate on the local alias to allow inlining. - */ - if (!n2->can_be_discarded_p ()) - { - cgraph_node *alias; - alias = dyn_cast (n2->noninterposable_alias ()); - if (alias) - n2 = alias; - } - nconverted++; - e->make_speculative (n2, - e->count.apply_probability ( - item.target_probability), - speculative_id, - item.target_probability); - update = true; - speculative_id++; - speculative_found = true; + "Function with profile-id %i not found.\n", + item.target_id); + nunknown++; } } - else - { - if (dump_file) - fprintf (dump_file, "Function with profile-id %i not found.\n", - item.target_id); - nunknown++; - } - } - if (speculative_found) - e->indirect_info->num_speculative_call_targets = speculative_id; } - } - if (update) - ipa_update_overall_fn_summary (n); - } + } + if (update) + ipa_update_overall_fn_summary (n); + } if (node_map_initialized) del_node_map (); if (dump_file && nindirect)