public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Kewen.Lin" <linkw@linux.ibm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Jan Hubicka <hubicka@ucw.cz>,
	Richard Biener <richard.guenther@gmail.com>,
	Richard Sandiford <richard.sandiford@arm.com>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	Peter Bergner <bergner@linux.ibm.com>
Subject: [PATCH v2] predict: Adjust optimize_function_for_size_p [PR105818]
Date: Wed, 30 Nov 2022 16:30:44 +0800	[thread overview]
Message-ID: <bbef44d3-fc72-ca13-d29c-2635e8b9d7b1@linux.ibm.com> (raw)

Hi,

Function optimize_function_for_size_p returns OPTIMIZE_SIZE_NO
if fun->decl is not null but no cgraph node is available for it.
As PR105818 shows, this could cause unexpected consequence.  For
the case in PR105818, when parsing bar decl in function foo, the
cfun is the function structure for foo, for which there is no
cgraph node, so it returns OPTIMIZE_SIZE_NO.  But it's incorrect
since the context is to optimize for size, the flag optimize_size
is true.

The patch is to make optimize_function_for_size_p to check
opt_for_fn (fun->decl, optimize_size) further when fun->decl
is available but no cgraph node, it's just like what function
cgraph_node::optimize_for_size_p does at its first step.

One regression failure got exposed on aarch64-linux-gnu:

PASS->FAIL: gcc.dg/guality/pr54693-2.c   -Os \
	    -DPREVENT_OPTIMIZATION  line 21 x == 10 - i

The difference comes from the macro LOGICAL_OP_NON_SHORT_CIRCUIT
used in function fold_range_test during c parsing, it uses
optimize_function_for_speed_p which is equal to the invertion
of optimize_function_for_size_p.  At that time cfun->decl is valid
but no cgraph node for it, w/o this patch function
optimize_function_for_speed_p returns true eventually, while it
returns false with this patch.  Since the command line option -Os
is specified, there is no reason to interpret it as "for speed".
I think this failure is expected and adjust the test case
accordingly.

v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596628.html

Comparing with v1, v2 adopts opt_for_fn (fun->decl, optimize_size)
instead of optimize_size as Honza's previous comments.

Besides, the reply to Honza's question "Why exactly PR105818 hits
the flag change issue?" was at the link:
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596667.html

Bootstrapped and regtested on x86_64-redhat-linux,
aarch64-linux-gnu and powerpc64{,le}-linux-gnu.

Is it for trunk?

BR,
Kewen
-----
	PR middle-end/105818

gcc/ChangeLog:

	* predict.cc (optimize_function_for_size_p): Further check
	optimize_size of fun->decl when it is valid but no cgraph node.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/pr105818.c: New test.
	* gcc.dg/guality/pr54693-2.c: Adjust for aarch64.
---
 gcc/predict.cc                              |  3 ++-
 gcc/testsuite/gcc.dg/guality/pr54693-2.c    |  2 +-
 gcc/testsuite/gcc.target/powerpc/pr105818.c | 11 +++++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr105818.c

diff --git a/gcc/predict.cc b/gcc/predict.cc
index 1bc7ab94454..ecb4aabc9df 100644
--- a/gcc/predict.cc
+++ b/gcc/predict.cc
@@ -268,7 +268,8 @@ optimize_function_for_size_p (struct function *fun)
   cgraph_node *n = cgraph_node::get (fun->decl);
   if (n)
     return n->optimize_for_size_p ();
-  return OPTIMIZE_SIZE_NO;
+  return opt_for_fn (fun->decl, optimize_size) ? OPTIMIZE_SIZE_MAX
+					       : OPTIMIZE_SIZE_NO;
 }

 /* Return true if function FUN should always be optimized for speed.  */
diff --git a/gcc/testsuite/gcc.dg/guality/pr54693-2.c b/gcc/testsuite/gcc.dg/guality/pr54693-2.c
index 68aa6c63d71..14ca94ad37d 100644
--- a/gcc/testsuite/gcc.dg/guality/pr54693-2.c
+++ b/gcc/testsuite/gcc.dg/guality/pr54693-2.c
@@ -17,7 +17,7 @@ foo (int x, int y, int z)
   int i = 0;
   while (x > 3 && y > 3 && z > 3)
     {		/* { dg-final { gdb-test .+2 "i" "v + 1" } } */
-		/* { dg-final { gdb-test .+1 "x" "10 - i" } } */
+		/* { dg-final { gdb-test .+1 "x" "10 - i" { xfail { aarch64*-*-* && { any-opts "-Os" } } } } } */
       bar (i);	/* { dg-final { gdb-test . "y" "20 - 2 * i" } } */
 		/* { dg-final { gdb-test .-1 "z" "30 - 3 * i" { xfail { aarch64*-*-* && { any-opts "-fno-fat-lto-objects" "-Os" } } } } } */
       i++, x--, y -= 2, z -= 3;
diff --git a/gcc/testsuite/gcc.target/powerpc/pr105818.c b/gcc/testsuite/gcc.target/powerpc/pr105818.c
new file mode 100644
index 00000000000..679647e189d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr105818.c
@@ -0,0 +1,11 @@
+/* { dg-options "-Os -fno-tree-vectorize" } */
+
+/* Verify there is no ICE.  */
+
+#pragma GCC optimize "-fno-tree-vectorize"
+
+void
+foo (void)
+{
+  void bar (void);
+}
--
2.27.0

             reply	other threads:[~2022-11-30  8:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-30  8:30 Kewen.Lin [this message]
2022-12-14 11:27 ` PING^1 " Kewen.Lin
2022-12-14 13:22   ` Jan Hubicka
2022-12-14 13:55     ` Martin Liška
2022-12-15  8:33     ` Kewen.Lin

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=bbef44d3-fc72-ca13-d29c-2635e8b9d7b1@linux.ibm.com \
    --to=linkw@linux.ibm.com \
    --cc=bergner@linux.ibm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=richard.guenther@gmail.com \
    --cc=richard.sandiford@arm.com \
    --cc=segher@kernel.crashing.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).