From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x229.google.com (mail-lj1-x229.google.com [IPv6:2a00:1450:4864:20::229]) by sourceware.org (Postfix) with ESMTPS id 971113858C5E for ; Fri, 3 Feb 2023 07:30:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 971113858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x229.google.com with SMTP id y19so4383404ljq.7 for ; Thu, 02 Feb 2023 23:30:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=R/VoWQscLKgmBhTKqdAqAfEBfuxyP3kkbPEVoGj8fGE=; b=PKJynhWA2KS1ltxRBmuC2SzUeQEBV9JkOPprIWobQyzEVJCe04ljK7u08pfD8MUqrM CsUTfEwNjjSxPHInghxHuBeU4GEMB9Zy+JDGQbwEoWjeAfOi0G2Eoky+Wizb7NPFZbzO gw+IDPX1e5LIKI5HOq+a4Qg+jXDgXC0heD/L2PdzyUoUCwQjPc6Iso2+mgTS0K61trvu MHCUyDEMXcbVmKXqyAT4th29M6piyANWhHi0HQMIAySvXXZfcAJ/4w1W3wnoOlruo/j5 CSatWSwooRTbs3MfHQpFVkNy7WKlbQTVj6edaN8Ygk1CsXMMWadisI7JHyb3Fd7TZGTn S/xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=R/VoWQscLKgmBhTKqdAqAfEBfuxyP3kkbPEVoGj8fGE=; b=B6RSOwTIjPSiAlKtQPiTlPhq9bsY20540557CLui5gAlWPk6rVu92Pdf9AdhDreRpu 5LNSkDqTHBkhLXtTvKg+vP6KWiZ1mS2AwfwqiNkh2VkqUtAPpGvk5AUIOxq7eiiqA2n7 1x5hrigzhz5gG5H4hlak1qo200QWKFERRIFkKvavlH0GjBwmAXTzTyyl5OfLwkJIRJQq ULVmQBpCePZrkbEd57NvgcYN4t96bufN+r7hbcYu0h06FuSSQFkxu417bbnmLIaeCETZ 3GJW0gaQjvzJvEwn5ZvjOqMXfdXB0zKF1t0wVFFd5y0A9xKVPWXaebTx7nWRIYRPMV2o 92KA== X-Gm-Message-State: AO0yUKWpD2waLH4vAYM4DF416p+1OFp1a/4RCrCsUlqR0UJJNzUt4ijf 3xGJzQXn5TQc42ZBnlMAzZSEQR+JIDuFPGsHDO8= X-Google-Smtp-Source: AK7set+htmqxwNSq4+3KqddOo1wCJtkxbB9ittcLPxIZHgR4GfAaW/zdziGf2hmcFt1/QNXx+DdrLKL9hHrwU8S3eA0= X-Received: by 2002:a2e:9b95:0:b0:290:5179:a344 with SMTP id z21-20020a2e9b95000000b002905179a344mr1425352lji.47.1675409434941; Thu, 02 Feb 2023 23:30:34 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Fri, 3 Feb 2023 08:30:22 +0100 Message-ID: Subject: Re: [PATCH] ipa: Avoid invalid gimple when IPA-CP and IPA-SRA disagree on types (108384) To: Martin Jambor Cc: GCC Patches , Jan Hubicka Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Feb 2, 2023 at 5:20 PM Martin Jambor wrote: > > Hi, > > when the compiled program contains type mismatches between callers and > callees when it comes to a parameter, IPA-CP can try to propagate one > constant from callers while IPA-SRA may try to split a parameter > expecting a value of a different size on the same offset. This then > currently leads to creation of a VIEW_CONVERT_EXPR with mismatching > type sizes of LHS and RHS which is correctly flagged by the GIMPLE > verifier as invalid. > > It seems that the best course of action is to try and avoid the > situation altogether and so this patch adds a check to IPA-SRA that > peeks into the result of IPA-CP and when it sees a value on the same > offset but with a mismatching size, it just decides to leave that > particular parameter be. > > Bootstrapped and tested on x86_64-linux, OK for master? OK. I suppose there are guards elsewhere that never lets a non-UHWI size type (like variable size or poly-int-size) through any of the SRA or CP lattices? Thanks, Richard. > Thanks, > > Martin > > > gcc/ChangeLog: > > 2023-02-02 Martin Jambor > > PR ipa/108384 > * ipa-sra.cc (push_param_adjustments_for_index): Remove a size check > when comparing to an IPA-CP value. > (dump_list_of_param_indices): New function. > (adjust_parameter_descriptions): Check for mismatching IPA-CP values. > Dump removed candidates using dump_list_of_param_indices. > * ipa-param-manipulation.cc > (ipa_param_body_adjustments::modify_expression): Add assert checking > sizes of a VIEW_CONVERT_EXPR will match. > (ipa_param_body_adjustments::modify_assignment): Likewise. > > gcc/testsuite/ChangeLog: > > 2023-02-02 Martin Jambor > > PR ipa/108384 > * gcc.dg/ipa/pr108384.c: New test. > --- > gcc/ipa-param-manipulation.cc | 4 ++ > gcc/ipa-sra.cc | 66 ++++++++++++++++++++--------- > gcc/testsuite/gcc.dg/ipa/pr108384.c | 25 +++++++++++ > 3 files changed, 76 insertions(+), 19 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr108384.c > > diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc > index 1de9ca2ceb8..42488ee09c3 100644 > --- a/gcc/ipa-param-manipulation.cc > +++ b/gcc/ipa-param-manipulation.cc > @@ -1857,6 +1857,8 @@ ipa_param_body_adjustments::modify_expression (tree *expr_p, bool convert) > if (convert && !useless_type_conversion_p (TREE_TYPE (expr), > TREE_TYPE (repl))) > { > + gcc_checking_assert (tree_to_shwi (TYPE_SIZE (TREE_TYPE (expr))) > + == tree_to_shwi (TYPE_SIZE (TREE_TYPE (repl)))); > tree vce = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (expr), repl); > *expr_p = vce; > } > @@ -1900,6 +1902,8 @@ ipa_param_body_adjustments::modify_assignment (gimple *stmt, > } > else > { > + gcc_checking_assert (tree_to_shwi (TYPE_SIZE (TREE_TYPE (*lhs_p))) > + == tree_to_shwi (TYPE_SIZE (TREE_TYPE (*rhs_p)))); > tree new_rhs = fold_build1_loc (gimple_location (stmt), > VIEW_CONVERT_EXPR, TREE_TYPE (*lhs_p), > *rhs_p); > diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc > index 81b75910db1..7a2b4dc8608 100644 > --- a/gcc/ipa-sra.cc > +++ b/gcc/ipa-sra.cc > @@ -3989,9 +3989,7 @@ push_param_adjustments_for_index (isra_func_summary *ifs, unsigned base_index, > { > ipa_argagg_value_list avl (ipcp_ts); > tree value = avl.get_value (base_index, pa->unit_offset); > - if (value > - && (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (value))) / BITS_PER_UNIT > - == pa->unit_size)) > + if (value) > { > if (dump_file) > fprintf (dump_file, " - omitting component at byte " > @@ -4130,6 +4128,22 @@ process_isra_node_results (cgraph_node *node, > callers.release (); > } > > +/* If INDICES is not empty, dump a combination of NODE's dump_name and MSG > + followed by the list of numbers in INDICES. */ > + > +static void > +dump_list_of_param_indices (const cgraph_node *node, const char* msg, > + const vec &indices) > +{ > + if (indices.is_empty ()) > + return; > + fprintf (dump_file, "The following parameters of %s %s:", node->dump_name (), > + msg); > + for (unsigned i : indices) > + fprintf (dump_file, " %u", i); > + fprintf (dump_file, "\n"); > +} > + > /* Check which parameters of NODE described by IFS have survived until IPA-SRA > and disable transformations for those which have not or which should not > transformed because the associated debug counter reached its limit. Return > @@ -4153,6 +4167,7 @@ adjust_parameter_descriptions (cgraph_node *node, isra_func_summary *ifs) > check_surviving = true; > cinfo->param_adjustments->get_surviving_params (&surviving_params); > } > + ipcp_transformation *ipcp_ts = ipcp_get_transformation_summary (node); > auto_vec dump_dead_indices; > auto_vec dump_bad_cond_indices; > for (unsigned i = 0; i < len; i++) > @@ -4202,27 +4217,40 @@ adjust_parameter_descriptions (cgraph_node *node, isra_func_summary *ifs) > if (size_would_violate_limit_p (desc, desc->size_reached)) > desc->split_candidate = false; > } > + > + /* Avoid ICEs on size-mismatched VIEW_CONVERT_EXPRs when callers and > + callees don't agree on types in aggregates and we try to do both > + IPA-CP and IPA-SRA. */ > + if (ipcp_ts && desc->split_candidate) > + { > + ipa_argagg_value_list avl (ipcp_ts); > + for (const param_access *pa : desc->accesses) > + { > + if (!pa->certain) > + continue; > + tree value = avl.get_value (i, pa->unit_offset); > + if (value > + && ((tree_to_uhwi (TYPE_SIZE (TREE_TYPE (value))) > + / BITS_PER_UNIT) > + != pa->unit_size)) > + { > + desc->split_candidate = false; > + if (dump_file && (dump_flags & TDF_DETAILS)) > + dump_dead_indices.safe_push (i); > + break; > + } > + } > + } > + > if (desc->locally_unused || desc->split_candidate) > ret = false; > } > } > > - if (!dump_dead_indices.is_empty ()) > - { > - fprintf (dump_file, "The following parameters of %s are dead on arrival:", > - node->dump_name ()); > - for (unsigned i : dump_dead_indices) > - fprintf (dump_file, " %u", i); > - fprintf (dump_file, "\n"); > - } > - if (!dump_bad_cond_indices.is_empty ()) > - { > - fprintf (dump_file, "The following parameters of %s are not safe to " > - "derefernce in all callers:", node->dump_name ()); > - for (unsigned i : dump_bad_cond_indices) > - fprintf (dump_file, " %u", i); > - fprintf (dump_file, "\n"); > - } > + dump_list_of_param_indices (node, "are dead on arrival or have a type " > + "mismatch with IPA-CP", dump_dead_indices); > + dump_list_of_param_indices (node, "are not safe to derefernce in all callers", > + dump_bad_cond_indices); > > return ret; > } > diff --git a/gcc/testsuite/gcc.dg/ipa/pr108384.c b/gcc/testsuite/gcc.dg/ipa/pr108384.c > new file mode 100644 > index 00000000000..2b714aa78b1 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr108384.c > @@ -0,0 +1,25 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O3" } */ > + > +struct S0 { > + int f0; > + short f1; > + unsigned f2 : 7; > + short f3; > +} func_2_l_27; > +int *g_389; > +int safe_sub_func_int16_t_s_s(void); > +void safe_lshift_func_uint8_t_u_s(int); > +void func_23(struct S0 p_24, struct S0 p_25) { > + int *l_1051 = g_389; > + if (safe_sub_func_int16_t_s_s()) > + for (;;) > + safe_lshift_func_uint8_t_u_s(p_24.f1); > + *l_1051 = p_25.f0; > +} > +void func_2(void) { > + struct S0 l_26[2]; > + l_26[1].f0 = 4; > + ((long long*)&l_26)[2] = 25770065925; > + func_23(l_26[1], func_2_l_27); > +} > -- > 2.39.0 >