public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Martin Jambor <jamborm@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r14-9794] ipa: Avoid duplicate replacements in IPA-SRA transformation phase
Date: Thu,  4 Apr 2024 20:59:45 +0000 (GMT)	[thread overview]
Message-ID: <20240404205945.DF92B3858C98@sourceware.org> (raw)

https://gcc.gnu.org/g:ca56b43105fc09021ec445f1978a17cd85ae5e0c

commit r14-9794-gca56b43105fc09021ec445f1978a17cd85ae5e0c
Author: Martin Jambor <mjambor@suse.cz>
Date:   Thu Apr 4 22:46:16 2024 +0200

    ipa: Avoid duplicate replacements in IPA-SRA transformation phase
    
    When the analysis part of IPA-SRA figures out that it would split out
    a scalar part of an aggregate which is known by IPA-CP to contain a
    known constant, it skips it knowing that the transformation part looks
    at IPA-CP aggregate results too and does the right thing (which can
    include doing the propagation in GIMPLE because that is the last
    moment the parameter exists).
    
    However, when IPA-SRA wants to split out a smaller aggregate out
    of an aggregate, which happens to be of the same size as a known
    scalar constant at the same offset, the transformation bit fails to
    recognize the situation, tries to do both splitting and constant
    propagation and in PR 111571 testcase creates a nonsensical call
    statement on which the call redirection then ICEs.
    
    Fixed by making sure we don't try to do two replacements of the same
    part of the same parameter.
    
    The look-up among replacements requires these are sorted and this
    patch just sorts them if they are not already sorted before each new
    look-up.  The worst number of sortings that can happen is number of
    parameters which are both split and have aggregate constants times
    param_ipa_max_agg_items (default 16).  I don't think complicating the
    source code to optimize for this unlikely case is worth it but if need
    be, it can of course be done.
    
    gcc/ChangeLog:
    
    2024-03-15  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/111571
            * ipa-param-manipulation.cc
            (ipa_param_body_adjustments::common_initialization): Avoid creating
            duplicate replacement entries.
    
    gcc/testsuite/ChangeLog:
    
    2024-03-15  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/111571
            * gcc.dg/ipa/pr111571.c: New test.

Diff:
---
 gcc/ipa-param-manipulation.cc       | 16 ++++++++++++++++
 gcc/testsuite/gcc.dg/ipa/pr111571.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc
index 3e0df6a6f77..f4b5e850c2b 100644
--- a/gcc/ipa-param-manipulation.cc
+++ b/gcc/ipa-param-manipulation.cc
@@ -1525,6 +1525,22 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl,
 		     replacement with a constant (for split aggregates passed
 		     by value).  */
 
+		  if (split[parm_num])
+		    {
+		      /* We must be careful not to add a duplicate
+			 replacement. */
+		      sort_replacements ();
+		      ipa_param_body_replacement *pbr
+			= lookup_replacement_1 (m_oparms[parm_num],
+						av.unit_offset);
+		      if (pbr)
+			{
+			  /* Otherwise IPA-SRA should have bailed out.  */
+			  gcc_assert (AGGREGATE_TYPE_P (TREE_TYPE (pbr->repl)));
+			  continue;
+			}
+		    }
+
 		  tree repl;
 		  if (av.by_ref)
 		    repl = av.value;
diff --git a/gcc/testsuite/gcc.dg/ipa/pr111571.c b/gcc/testsuite/gcc.dg/ipa/pr111571.c
new file mode 100644
index 00000000000..2a4adc608db
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr111571.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2"  } */
+
+struct a {
+  int b;
+};
+struct c {
+  long d;
+  struct a e;
+  long f;
+};
+int g, h, i;
+int j() {return 0;}
+static void k(struct a l, int p) {
+  if (h)
+    g = 0;
+  for (; g; g = j())
+    if (l.b)
+      break;
+}
+static void m(struct c l) {
+  k(l.e, l.f);
+  for (;; --i)
+    ;
+}
+int main() {
+  struct c n = {10, 9};
+  m(n);
+}

                 reply	other threads:[~2024-04-04 20:59 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240404205945.DF92B3858C98@sourceware.org \
    --to=jamborm@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).