Bug 38747 - [4.4 Regression] Wrong code due to VIEW_CONVERT_EXPR
Summary: [4.4 Regression] Wrong code due to 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.5.0
Assignee: Not yet assigned to anyone
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: patch, wrong-code
Depends on:
Blocks: 36143 38748
  Show dependency treegraph
 
Reported: 2009-01-06 21:30 UTC by Richard Biener
Modified: 2012-03-13 13:04 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.3.2 4.5.0
Known to fail:
Last reconfirmed: 2009-01-06 21:49:26


Attachments
Patch which should fix this and PR 38748 (902 bytes, patch)
2009-01-15 23:26 UTC, Andrew Pinski
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2009-01-06 21:30:25 UTC
struct S { float f; };
int __attribute__((noinline))
foo (int *r, struct S *p)
{
  int *q = (int *)&p->f;
  int i = *q;
  *r = 0;
  return i + *q;
}
extern void abort (void);
int main()
{
  int i = 1;
  if (foo (&i, (struct S *)&i) != 1)
    abort ();
}
Comment 1 Andrew Pinski 2009-01-06 21:49:25 UTC
Mine.  I am working on this with the other VCE issue.
Comment 2 Andrew Pinski 2009-01-15 23:26:56 UTC
Created attachment 17111 [details]
Patch which should fix this and PR 38748

This patch still creates VCE for INDIRECT_REF based but only if the aliasing sets are the same.  
This is to allow for the following to be optimized:
struct S { unsigned f; };
int __attribute__((noinline))
foo (float *r, struct S *p)
{
  int *q = (int *)&p->f;
  int i = *q;
  *r = 0.0;
  return i + *q;
}

Without a need for &p->f in the IR.
Comment 3 Richard Biener 2009-01-16 09:23:11 UTC
I think you should simply do the check for an indirect base only if the folding
of the view-convert-expr ended up in a view-convert-expr.  Playing with alias-sets
here will still cause these nested v-c-es with -fno-strict-aliasing and loads
of unwanted (and likely unprofitable) v-c-es.  Your testcase should be still
optimized then.
Comment 4 Richard Biener 2009-01-20 17:10:54 UTC
Fixed.
Comment 5 Richard Biener 2009-01-20 17:10:57 UTC
Subject: Bug 38747

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 6 hjl@gcc.gnu.org 2009-01-30 17:32:05 UTC
Subject: Bug 38747

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

Comment 7 Richard Biener 2009-05-20 15:26:05 UTC
On a second thought this transformation is even wrong for non-indirect references.

extern "C" void abort (void);
inline void *operator new (__SIZE_TYPE__, void *__p) throw () { return __p; }

int __attribute__((noinline))
foo(void)
{
  float f = 0;
  int *i = new (&f) int (1);
  return *(int *)&f;
}

int main()
{
  if (foo() != 1)
    abort ();
  return 0;
}

it is wrong to turn the float read into an int read converted via a VIEW_CONVERT_EXPR.

I will remove all this VIEW_CONVERT_EXPR generation again.
Comment 8 Richard Biener 2009-05-20 15:26:32 UTC
Mine.
Comment 9 Richard Biener 2009-09-24 13:47:55 UTC
Subject: Bug 38747

Author: rguenth
Date: Thu Sep 24 13:47:26 2009
New Revision: 152122

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=152122
Log:
2009-09-24  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/36143
	PR tree-optimization/38747
	* tree-ssa-forwprop.c (forward_propagate_addr_expr_1): Only
	create VIEW_CONVERT_EXPRs for TBAA compatible accesses.

	* gcc.dg/tree-ssa/fre-vce-1.c: XFAIL.
	* gcc.dg/tree-ssa/forwprop-6.c: Likewise.
	* g++.dg/torture/pr38747.C: New testcase.
	* g++.dg/tree-ssa/pr19637.C: Un-XFAIL.

Added:
    trunk/gcc/testsuite/g++.dg/torture/pr38747.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/tree-ssa/pr19637.C
    trunk/gcc/testsuite/gcc.dg/tree-ssa/forwprop-6.c
    trunk/gcc/testsuite/gcc.dg/tree-ssa/fre-vce-1.c
    trunk/gcc/tree-ssa-forwprop.c

Comment 10 Richard Biener 2009-09-24 13:48:37 UTC
Fixed on trunk.  Trunk doesn't necessarily handle testcases like this correctly
anyway, so I am not considering in backporting the change ATM.
Comment 11 Jakub Jelinek 2012-03-13 13:04:27 UTC
Fixed in 4.5+, 4.4 is no longer supported.