Re: [PATCH] Fix tree-opt/PR106087: ICE with inline-asm with multiple output and assigned only static vars

2022-07-08 Thread Richard Biener via Gcc-patches
On Fri, Jul 8, 2022 at 6:33 AM apinski--- via Gcc-patches
 wrote:
>
> From: Andrew Pinski 
>
> The problem here is that when we mark the ssa name that was referenced in the 
> now removed
> dead store (to a write only static variable), the inline-asm would also be 
> removed
> even though it was defining another ssa name. This fixes the problem by 
> checking
> to make sure that the statement was only defining one ssa name.
>
> OK? Bootstrapped and tested on x86_64 with no regressions.
>
> PR tree-optimization/106087
>
> gcc/ChangeLog:
>
> * tree-ssa-dce.cc (simple_dce_from_worklist): Check
> to make sure the statement is only defining one operand.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.c-torture/compile/inline-asm-1.c: New test.
> ---
>  gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c | 14 ++
>  gcc/tree-ssa-dce.cc|  5 +
>  2 files changed, 19 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c
>
> diff --git a/gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c 
> b/gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c
> new file mode 100644
> index 000..0044cb761b6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c
> @@ -0,0 +1,14 @@
> +/* PR tree-opt/106087,
> +   simple_dce_from_worklist would delete the
> +   inline-asm when it was still being referenced
> +   by the other ssa name. */
> +
> +static int t;
> +
> +int f(void)
> +{
> +  int tt, tt1;
> +  asm("":"=r"(tt), "=r"(tt1));
> +  t = tt1;
> +  return tt;
> +}
> diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
> index bc533582673..602cdb30ceb 100644
> --- a/gcc/tree-ssa-dce.cc
> +++ b/gcc/tree-ssa-dce.cc
> @@ -2061,6 +2061,11 @@ simple_dce_from_worklist (bitmap worklist)
>if (gimple_has_side_effects (t))
> continue;
>
> +  /* The defining statement needs to be defining one this name. */

only this name?

> +  if (!is_a(t)

I suppose we could turn this into is_a(t) since that's
the only stmt kind with multiple (non-virtual) definitions?

OK with those changes.

> + && !single_ssa_def_operand (t, SSA_OP_DEF))
> +   continue;
> +
>/* Don't remove statements that are needed for non-call
>  eh to work.  */
>if (stmt_unremovable_because_of_non_call_eh_p (cfun, t))
> --
> 2.17.1
>


[PATCH] Fix tree-opt/PR106087: ICE with inline-asm with multiple output and assigned only static vars

2022-07-07 Thread apinski--- via Gcc-patches
From: Andrew Pinski 

The problem here is that when we mark the ssa name that was referenced in the 
now removed
dead store (to a write only static variable), the inline-asm would also be 
removed
even though it was defining another ssa name. This fixes the problem by checking
to make sure that the statement was only defining one ssa name.

OK? Bootstrapped and tested on x86_64 with no regressions.

PR tree-optimization/106087

gcc/ChangeLog:

* tree-ssa-dce.cc (simple_dce_from_worklist): Check
to make sure the statement is only defining one operand.

gcc/testsuite/ChangeLog:

* gcc.c-torture/compile/inline-asm-1.c: New test.
---
 gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c | 14 ++
 gcc/tree-ssa-dce.cc|  5 +
 2 files changed, 19 insertions(+)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c

diff --git a/gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c 
b/gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c
new file mode 100644
index 000..0044cb761b6
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/inline-asm-1.c
@@ -0,0 +1,14 @@
+/* PR tree-opt/106087,
+   simple_dce_from_worklist would delete the
+   inline-asm when it was still being referenced
+   by the other ssa name. */
+
+static int t;
+
+int f(void)
+{
+  int tt, tt1;
+  asm("":"=r"(tt), "=r"(tt1));
+  t = tt1;
+  return tt;
+}
diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index bc533582673..602cdb30ceb 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -2061,6 +2061,11 @@ simple_dce_from_worklist (bitmap worklist)
   if (gimple_has_side_effects (t))
continue;
 
+  /* The defining statement needs to be defining one this name. */
+  if (!is_a(t)
+ && !single_ssa_def_operand (t, SSA_OP_DEF))
+   continue;
+
   /* Don't remove statements that are needed for non-call
 eh to work.  */
   if (stmt_unremovable_because_of_non_call_eh_p (cfun, t))
-- 
2.17.1