Bug 38748 - [4.4 Regression] Missed FRE because of VIEW_CONVERT_EXPR
Summary: [4.4 Regression] Missed FRE because of VIEW_CONVERT_EXPR
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.4.0
: P2 normal
Target Milestone: 4.4.0
Assignee: Andrew Pinski
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: missed-optimization, patch
Depends on: 38747
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-06 21:32 UTC by Richard Biener
Modified: 2009-01-20 17:11 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.2
Known to fail:
Last reconfirmed: 2009-01-13 01:09:36


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2009-01-06 21:32:33 UTC
The load from *q should be CSEd but it isn't because tree forwprop introduces
a VIEW_CONVERT_EXPR from a load of p->f which conflicts with the store
through *r.

/* { dg-do "compile" } */
/* { dg-options "-O -fdump-tree-fre" } */

struct S { float f; };
int __attribute__((noinline))
foo (float *r, struct S *p)
{
  int *q = (int *)&p->f;
  int i = *q;
  *r = 0.0;
  return i + *q;
}

/* { dg-final { scan-tree-dump-times "\\\*q" 1 "fre" } } */
/* { dg-final { cleanup-tree-dump "fre" } } */
Comment 1 Richard Biener 2009-01-06 21:33:08 UTC
Related to PR38747.
Comment 2 Richard Biener 2009-01-11 14:29:19 UTC
This hits tramp3d a lot, we end up with

D.919534 = VIEW_CONVERT_EXPR<const struct Loc>(D.919509.D.78313.D.78008).D.78313.D.78008.domain_m[0].D.77181.D.47055.D.46936.domain_m;

vs. D.919509.D.78313.D.78008...

the following fixes it:

Index: gcc/tree-ssa-forwprop.c
===================================================================
--- gcc/tree-ssa-forwprop.c	(revision 143251)
+++ gcc/tree-ssa-forwprop.c	(working copy)
@@ -775,10 +775,13 @@ forward_propagate_addr_expr_1 (tree name
       && !TYPE_VOLATILE (TREE_TYPE (rhs))
       && !TYPE_VOLATILE (TREE_TYPE (TREE_OPERAND (def_rhs, 0)))
       && operand_equal_p (TYPE_SIZE (TREE_TYPE (rhs)),
-			  TYPE_SIZE (TREE_TYPE (TREE_OPERAND (def_rhs, 0))), 0)) 
+			  TYPE_SIZE (TREE_TYPE (TREE_OPERAND (def_rhs, 0))), 0)
+      && (rhs2 = get_base_address (TREE_OPERAND (def_rhs, 0))) != NULL_TREE
+      && !INDIRECT_REF_P (rhs2))
    {
      tree new_rhs = unshare_expr (TREE_OPERAND (def_rhs, 0));
      new_rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (rhs), new_rhs);
+     STRIP_USELESS_TYPE_CONVERSION (new_rhs);
      if (TREE_CODE (new_rhs) != VIEW_CONVERT_EXPR)
        {
 	 /* If we have folded the VIEW_CONVERT_EXPR then the result is only
Comment 3 Richard Biener 2009-01-11 14:41:05 UTC
I can even see things like

SR.20836 = VIEW_CONVERT_EXPR<const struct Interval>(VIEW_CONVERT_EXPR<const struct Interval>(this->domain_m.D.115040.D.114739.domain_m[1].D.111565.D.45708.D.45522).D.45708.D.45522).D.45708.D.45522.domain_m[0];

in final cleanup :/
Comment 4 Andrew Pinski 2009-01-13 01:09:36 UTC
We should be able to fold:
VIEW_CONVERT_EXPR<const struct Loc>(D.919509.D.78313.D.78008)

Into just D.919509 .

The indirect issue is a different one which needs to be fixed anyways as it causes aliasing issues which were not in the original code in some cases.
Comment 5 rguenther@suse.de 2009-01-13 08:55:08 UTC
Subject: Re:  [4.4 Regression] Missed FRE because
 of VIEW_CONVERT_EXPR

On Tue, 13 Jan 2009, pinskia at gcc dot gnu dot org wrote:

> ------- Comment #4 from pinskia at gcc dot gnu dot org  2009-01-13 01:09 -------
> We should be able to fold:
> VIEW_CONVERT_EXPR<const struct Loc>(D.919509.D.78313.D.78008)
> 
> Into just D.919509 .

True.  I tried a quick shot at it but it turns out to be slightly
complicated ;)

Richard.
Comment 6 Andrew Pinski 2009-01-15 23:16:36 UTC
The CSE of the load is only valid at -O2 and above (strict aliasing turned on).
Comment 7 Andrew Pinski 2009-01-15 23:30:40 UTC
PR 38747 has the best patch which allows most optimizations still but only disabling it for the invalid cases when the aliasing sets are not equal.
Take:
struct S { unsigned f; };
int __attribute__((noinline))
foo ( struct S *p)
{
  int *q = (int *)&p->f;
  int i = *q;
  return i + p->f;
}

We should be able to optimize this to just "tmp = p->f; return (int)(tmp + tmp);" which we do with my patch unlike the one in comment #2 which disables too many optimizations.
Comment 8 Richard Biener 2009-01-20 17:10:57 UTC
Subject: Bug 38748

Author: rguenth
Date: Tue Jan 20 17:10:40 2009
New Revision: 143523

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143523
Log:
2009-01-20  Andrew Pinski  <andrew_pinski@playstation.sony.com>
	Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38747
	PR tree-optimization/38748
	* tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Disable the VCE
	conversion if the base address is an indirect reference and the
	aliasing sets could cause issues.

	* gcc.dg/tree-ssa/struct-aliasing-1.c: New test.
	* gcc.dg/tree-ssa/struct-aliasing-2.c: Likewise.
	* gcc.c-torture/execute/struct-aliasing-1.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/struct-aliasing-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/struct-aliasing-1.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/struct-aliasing-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-forwprop.c

Comment 9 Richard Biener 2009-01-20 17:11:04 UTC
Fixed.
Comment 10 hjl@gcc.gnu.org 2009-01-30 17:32:05 UTC
Subject: Bug 38748

Author: hjl
Date: Fri Jan 30 17:31:24 2009
New Revision: 143798

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143798
Log:
2009-01-30  H.J. Lu  <hongjiu.lu@intel.com>

	2009-01-27  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38503
	* g++.dg/warn/Wstrict-aliasing-bogus-placement-new.C: New testcase.

	2009-01-26  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38745
	* g++.dg/torture/pr38745.C: New testcase.

	2009-01-26  Richard Guenther  <rguenther@suse.de>

	PR middle-end/38851
	* g++.dg/warn/Wuninitialized-1.C: New testcase.

	2009-01-20  Andrew Pinski  <andrew_pinski@playstation.sony.com>
		    Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38747
	PR tree-optimization/38748
	* gcc.dg/tree-ssa/struct-aliasing-1.c: New test.
	* gcc.c-torture/execute/struct-aliasing-1.c: Likewise.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/torture/pr38745.C
      - copied unchanged from r143797, trunk/gcc/testsuite/g++.dg/torture/pr38745.C
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/warn/Wstrict-aliasing-bogus-placement-new.C
      - copied unchanged from r143797, trunk/gcc/testsuite/g++.dg/warn/Wstrict-aliasing-bogus-placement-new.C
    branches/gcc-4_3-branch/gcc/testsuite/g++.dg/warn/Wuninitialized-1.C
      - copied unchanged from r143797, trunk/gcc/testsuite/g++.dg/warn/Wuninitialized-1.C
    branches/gcc-4_3-branch/gcc/testsuite/gcc.c-torture/execute/struct-aliasing-1.c
      - copied unchanged from r143797, trunk/gcc/testsuite/gcc.c-torture/execute/struct-aliasing-1.c
    branches/gcc-4_3-branch/gcc/testsuite/gcc.dg/tree-ssa/struct-aliasing-1.c
      - copied unchanged from r143797, trunk/gcc/testsuite/gcc.dg/tree-ssa/struct-aliasing-1.c
Modified:
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog