On Wed, 9 Mar 2022, Jakub Jelinek wrote:
> On Wed, Mar 09, 2022 at 11:07:21AM +0100, Richard Biener wrote:
> > The following fixes an ICE observed with a MEM_REF allows_mem asm
> > operand. There's code expecting INDIRECT_REFs that are now never
> > going to appear. The following simply treats all tcc_reference
> > operands the same.
>
> The INDIRECT_REF in there seems to be at least from 1995, but my limited
> understanding of what it wants to catch is operands that provably must
> live in memory. INDIRECT_REF (assuming *&*& is folded already) is
> such a case, MEM_REF could be but not always (there is the case of
> MEM_REF on &decl without TREE_ADDRESSABLE) but usually is,
> other handled_component_p depend on what their first operand is etc.
>
> I think there are two cases, one is an optimization (the && allows_mem
> case) where we don't want create a temporary (especially a huge one)
> if it clearly has to live in memory. Example where we mess this up:
> struct T { char b[1024]; };
> struct S { int a; struct T b; } s;
>
> void
> foo (void)
> {
> asm volatile ("# %0" : : "rm" (s.b));
> }
> where we create 1024 bytes long temporary on the stack, copy s.b to it
> and that is the operand live in memory.
> So perhaps for the allows_mem && case we could handle that way
> all BLKmode types (BLKmode must live in memory, no?) and otherwise
> look through all handled_component_p's and determine if the base
> will be in memory (addressable decl, MEM_REF except for ADDR of
> non-addressable decl, INDIRECT_REF, what else?).
Well, in theory you could have a MEM_REF<struct T> (&int_decl)
with a BLKmode struct T but SImode int_decl. So looking at the
mode isn't good enough I think. Note that the DECL_P case
does simply allow a non-MEM_P DECL_RTL when allows_mem and
only disallows the case of a REG with a different mode than
the type (but if it say a CONCAT it doesn't care). That was
my reasoning that any additional restrictions are not necessary.
Of course we might run into
if (! allows_reg && !MEM_P (op))
{
error_at (locus, "output number %d not directly
addressable", i);
error_seen = true;
}
then, so when !allows_reg it might be better to go the
temporary copy way. But then I fail to see how !allows_mem
is OK in that case ...
> And another case is where it is not an optimization, where we
> just can't ever create a temporary.
> One of those examples is
> || TREE_ADDRESSABLE (type)
> the if already has, I guess non-constant length (ok, SVE is ok)
> would be another.
> assign_temp tests !poly_int_tree_p (TYPE_SIZE_UNIT (type), &size),
> so perhaps add || !tree_fits_poly_int64_p (TYPE_SIZE_UNIT (type)) ?
Yes, and that's good enough for the testcase at hand. assign_temp
also handles some cases through max_int_size_in_bytes. I'm going
to leave the optimization above for a followup and will test the
following for now.
Richard.
>From 90066f8dc0353103b9e7276b7a3e18e925509ded Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Wed, 9 Mar 2022 10:55:49 +0100
Subject: [PATCH] middle-end/104786 - ICE with asm and VLA
To: gcc-patches@gcc.gnu.org
The following fixes an ICE observed with a MEM_REF allows_mem asm
operand referencing a VLA. The following makes sure to not attempt
to go the temporary creation way when we cannot.
2022-03-09 Richard Biener <rguent...@suse.de>
PR middle-end/104786
* cfgexpand.cc (expand_asm_stmt): Do not generate a copy
for VLAs without an upper size bound.
* gcc.dg/pr104786.c: New testcase.
---
gcc/cfgexpand.cc | 4 +++-
gcc/testsuite/gcc.dg/pr104786.c | 8 ++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr104786.c
diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 87536ec7ccd..271bbfb8fe9 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -3297,7 +3297,9 @@ expand_asm_stmt (gasm *stmt)
&& GET_MODE (DECL_RTL (val)) != TYPE_MODE (type)))
|| ! allows_reg
|| is_inout
- || TREE_ADDRESSABLE (type))
+ || TREE_ADDRESSABLE (type)
+ || (!tree_fits_poly_int64_p (type)
+ && !known_size_p (max_int_size_in_bytes (type))))
{
op = expand_expr (val, NULL_RTX, VOIDmode,
!allows_reg ? EXPAND_MEMORY : EXPAND_WRITE);
diff --git a/gcc/testsuite/gcc.dg/pr104786.c b/gcc/testsuite/gcc.dg/pr104786.c
new file mode 100644
index 00000000000..3076d236d21
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr104786.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-std=gnu90" } */
+
+void h(void *di, int num)
+{
+ char (*t)[num] = di;
+ __asm__ ("" : "=X"( *t));
+}
--
2.34.1
