public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>,
	Jeff Law <jeffreyalaw@gmail.com>,
	Eric Botcazou <botcazou@adacore.com>
Cc: gcc-patches@gcc.gnu.org, Roger Sayle <roger@nextmovesoftware.com>
Subject: [PATCH] expr: Fix up emit_push_insn [PR114552]
Date: Wed, 3 Apr 2024 09:40:55 +0200	[thread overview]
Message-ID: <Zg0IB+apdMZcKCy9@tucnak> (raw)

Hi!

r13-990 added optimizations in multiple spots to optimize during
expansion storing of constant initializers into targets.
In the load_register_parameters and expand_expr_real_1 cases,
it checks it has a tree as the source and so knows we are reading
that whole decl's value, so the code is fine as is, but in the
emit_push_insn case it checks for a MEM from which something
is pushed and checks for SYMBOL_REF as the MEM's address, but
still assumes the whole object is copied, which as the following
testcase shows might not always be the case.  In the testcase,
k is 6 bytes, then 2 bytes of padding, then another 4 bytes,
while the emit_push_insn wants to store just the 6 bytes.

The following patch simply verifies it is the whole initializer
that is being stored, I think that is best thing to do so late
in GCC 14 cycle as well for backporting.

For GCC 15, perhaps the code could stop requiring it must be at offset zero,
nor that the size is a subset, but could use
get_symbol_constant_value/fold_ctor_reference gimple-fold APIs to actually
extract just part of the initializer if we e.g. push just some subset
(of course, still verify that it is a subset).  For sizes which are power
of two bytes and we have some integer modes, we could use as type for
fold_ctor_reference corresponding integral types, otherwise dunno, punt
or use some structure (e.g. try to find one in the initializer?), whatever.
But even in the other spots it could perhaps handle loading of
COMPONENT_REFs or MEM_REFs from the .rodata vars.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-04-02  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/114552
	* expr.cc (emit_push_insn): Only use store_constructor for
	immediate_const_ctor_p if int_expr_size matches size.

	* gcc.c-torture/execute/pr114552.c: New test.

--- gcc/expr.cc.jj	2024-03-15 10:10:51.209237835 +0100
+++ gcc/expr.cc	2024-04-02 16:01:39.566744302 +0200
@@ -5466,6 +5466,7 @@ emit_push_insn (rtx x, machine_mode mode
 	  /* If source is a constant VAR_DECL with a simple constructor,
              store the constructor to the stack instead of moving it.  */
 	  const_tree decl;
+	  HOST_WIDE_INT sz;
 	  if (partial == 0
 	      && MEM_P (xinner)
 	      && SYMBOL_REF_P (XEXP (xinner, 0))
@@ -5473,9 +5474,11 @@ emit_push_insn (rtx x, machine_mode mode
 	      && VAR_P (decl)
 	      && TREE_READONLY (decl)
 	      && !TREE_SIDE_EFFECTS (decl)
-	      && immediate_const_ctor_p (DECL_INITIAL (decl), 2))
-	    store_constructor (DECL_INITIAL (decl), target, 0,
-			       int_expr_size (DECL_INITIAL (decl)), false);
+	      && immediate_const_ctor_p (DECL_INITIAL (decl), 2)
+	      && (sz = int_expr_size (DECL_INITIAL (decl))) > 0
+	      && CONST_INT_P (size)
+	      && INTVAL (size) == sz)
+	    store_constructor (DECL_INITIAL (decl), target, 0, sz, false);
 	  else
 	    emit_block_move (target, xinner, size, BLOCK_OP_CALL_PARM);
 	}
--- gcc/testsuite/gcc.c-torture/execute/pr114552.c.jj	2024-04-02 16:08:12.959366793 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr114552.c	2024-04-02 16:03:49.829963659 +0200
@@ -0,0 +1,24 @@
+/* PR middle-end/114552 */
+
+struct __attribute__((packed)) S { short b; int c; };
+struct T { struct S b; int e; };
+static const struct T k = { { 1, 0 }, 0 };
+
+__attribute__((noinline)) void
+foo (void)
+{
+  asm volatile ("" : : : "memory");
+}
+
+__attribute__((noinline)) void
+bar (struct S n)
+{
+  foo ();
+}
+
+int
+main ()
+{
+  bar (k.b);
+  return 0;
+}

	Jakub


             reply	other threads:[~2024-04-03  7:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03  7:40 Jakub Jelinek [this message]
2024-04-03  7:43 ` Richard Biener

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=Zg0IB+apdMZcKCy9@tucnak \
    --to=jakub@redhat.com \
    --cc=botcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jeffreyalaw@gmail.com \
    --cc=rguenther@suse.de \
    --cc=roger@nextmovesoftware.com \
    /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).