| Message ID | 6fa65cba-96b8-dba8-f8ef-015d458f0fc2@suse.cz |
|---|---|
| State | New |
| Headers | show |
> This is candidate patch for the PR, which do not create SAVE_EXPR trees for > already assigned SSA_NAMEs. > > Patch survives reg&bootstrap on x86_64-linux-gnu. > > Thoughts? This looks like a layering violation, save_expr is a GENERIC thing so invoking it on an SSA_NAME is weird. How does this happen?
On Thu, Jun 23, 2016 at 12:41:53PM +0200, Eric Botcazou wrote: > > This is candidate patch for the PR, which do not create SAVE_EXPR trees for > > already assigned SSA_NAMEs. > > > > Patch survives reg&bootstrap on x86_64-linux-gnu. > > > > Thoughts? > > This looks like a layering violation, save_expr is a GENERIC thing so invoking > it on an SSA_NAME is weird. How does this happen? The gimplifier has been changed recently to use anonymous SSA_NAMEs instead of temporary decls. And the gimplifier uses save_expr (which is a gimplifier function BTW) on both not gimplified at all as well as partially gimplified trees. Jakub
> The gimplifier has been changed recently to use anonymous SSA_NAMEs instead > of temporary decls. But the PR is a regression present since GCC 4.7... > And the gimplifier uses save_expr (which is a gimplifier function BTW) on > both not gimplified at all as well as partially gimplified trees. Are you confounding it with something else? Because save_expr is definitely not a gimplifier function, it's mostly used to build GENERIC trees. That being said, I can imagine it being invoked from the gimplifier, but I'd like to see the backtrace.
On Thu, Jun 23, 2016 at 1:14 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> The gimplifier has been changed recently to use anonymous SSA_NAMEs instead >> of temporary decls. > > But the PR is a regression present since GCC 4.7... > >> And the gimplifier uses save_expr (which is a gimplifier function BTW) on >> both not gimplified at all as well as partially gimplified trees. > > Are you confounding it with something else? Because save_expr is definitely > not a gimplifier function, it's mostly used to build GENERIC trees. That > being said, I can imagine it being invoked from the gimplifier, but I'd like > to see the backtrace. So the issue is that the inliner uses fold_convert: if (value && value != error_mark_node && !useless_type_conversion_p (TREE_TYPE (p), TREE_TYPE (value))) { /* If we can match up types by promotion/demotion do so. */ if (fold_convertible_p (TREE_TYPE (p), value)) rhs = fold_convert (TREE_TYPE (p), value); else and converting a _Complex double to a _Complex long double. That generates COMPLEX_EXPR <(long double) REALPART_EXPR <SAVE_EXPR <a.0_1>>, (long double) IMAGPART_EXPR <SAVE_EXPR <a.0_1>>> which in insert_init_stmt we insert into the IL w/o gimplifying it (thus the operand scanner ICEs). IMHO using fold-convert in this case is bogus and ideally the testcase should have been diagnosed. fold_convertible_p has a comment /* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. * but clearly it isn't generating just a NOP_EXPR (or VIEW_CONVERT_EXPR or other single operation) here. So that is the thing to fix. The way we build / insert the init stmts can also be improved by properly gimplifying the rhs first but of course that likely runs into the SAVE_EXPR case you mentioned. Richard. > -- > Eric Botcazou
From 91d01830302171b5cd53fa2f32cc881b2b50762f Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Wed, 22 Jun 2016 18:07:55 +0200 Subject: [PATCH] Do not emit SAVE_EXPR for already assigned SSA_NAMEs (PR71606). gcc/ChangeLog: 2016-06-22 Martin Liska <mliska@suse.cz> PR middle-end/71606 * tree.c (save_expr): Do not generate SAVE_EXPR if the argument is already an assigned SSA_NAME. gcc/testsuite/ChangeLog: 2016-06-22 Martin Liska <mliska@suse.cz> * gcc.dg/torture/pr71606.c: New test. --- gcc/testsuite/gcc.dg/torture/pr71606.c | 11 +++++++++++ gcc/tree.c | 3 +++ 2 files changed, 14 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/torture/pr71606.c diff --git a/gcc/testsuite/gcc.dg/torture/pr71606.c b/gcc/testsuite/gcc.dg/torture/pr71606.c new file mode 100644 index 0000000..b0cc26a --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr71606.c @@ -0,0 +1,11 @@ +_Complex a; +void fn1 (); + +int main () { + fn1 (a); + return 0; +} + +void fn1 (__complex__ long double p1) { + __imag__ p1 = 6.0L; +} diff --git a/gcc/tree.c b/gcc/tree.c index bc60190..344eb61 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -3340,6 +3340,9 @@ save_expr (tree expr) tree t = fold (expr); tree inner; + if (TREE_CODE (expr) == SSA_NAME && SSA_NAME_DEF_STMT (expr)) + return t; + /* If the tree evaluates to a constant, then we don't want to hide that fact (i.e. this allows further folding, and direct checks for constants). However, a read-only object that has side effects cannot be bypassed. -- 2.8.4