Bug 35164 - [4.3 regression] Unable to coalesce ab SSA_NAMEs
Summary: [4.3 regression] Unable to coalesce ab SSA_NAMEs
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P1 normal
Target Milestone: 4.3.0
Assignee: Richard Biener
URL:
Keywords: ice-on-valid-code
: 30604 35182 (view as bug list)
Depends on:
Blocks: 30604
  Show dependency treegraph
 
Reported: 2008-02-11 12:19 UTC by Jakub Jelinek
Modified: 2020-07-28 05:54 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-02-13 04:06:16


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2008-02-11 12:19:10 UTC
On x86_64 with -O2 the following testcase ICEs with:
D.11907_556(ab) and  D.11907_14(ab)
rh432296.C: In function `void foo(std::vector<B<D>, std::allocator<B<D> > >&)':
rh432296.C:52: internal compiler error: SSA corruption
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

That looks similar to the inlining AB problem Jan fixed, but this is with current trunk.

Sorry for the left-over #include <vector>, didn't get to minimize this more yet.
#include <vector>

struct A
{
  A ();
  virtual ~A ();
  inline void incRef ();
  inline void decRef ();
  virtual void free () {}
  int m_refCount;
};
void A::incRef () { m_refCount++; }
void A::decRef () { if (!m_refCount) { free (); return; } m_refCount--; }
template <class T> struct B;
struct C : public A
{
  static C *alloc ();
};
template <class T>
struct B
{
  typedef T DataT;
  B () : ptr (T::alloc ()) { }
  B (const B<T>& a) : ptr (a.get ()) { incRef (); }
  B (T *a_ptr) : ptr (a_ptr) { incRef (); }
  ~B () { decRef (); }
  B& operator= (const B<T>& a) { if (a.get () != this->get ()) { decRef (); ptr = a.get (); incRef (); } return *this; }
  template<class U>
  operator B<U> () const { return B<U> (ptr); }
  template<class U>
  operator B<const U> () const { return B<const U> (ptr); }
  T* operator-> () const { return ptr; }
  T* get () const { return ptr; }
  void decRef () const { if (ptr != 0) ptr->decRef (); }
  void incRef () const { if (ptr != 0) ptr->incRef (); }
  T *ptr;
};
struct D : public C
{
  template <class T>
  inline void foo (const B<T> & x) { d.resize (1); d[0] = x; }
  std::vector<B <C> > d;
};
struct E : public C
{
  static E *alloc ();
};
struct F : public D
{
  static F *alloc ();
};
void foo (std::vector<B<D> > & x)
{
  for (int i = 0; i < 2; ++i)
    {
      B<F> l;
      B<E> m;
      l->foo (m);
      x.push_back (l);
    }
}
Comment 1 Jakub Jelinek 2008-02-12 12:00:15 UTC
But this one doesn't seem to be inlining related.  Until vrp1 pass the var in question (D.20903) has just one SSA_NAME (D.20903_14) in the whole function, non-ab, initialized at the beginning of the loop and nothing before the loop or in loop header can throw.  vrp introduces a lot of new SSA_NAMEs for this variable, now (ab) marked in the dump, but except one are of the form:
# D.20903_627(ab) = PHI <D.20903_14(ab)(5)>
# D.20903_628(ab) = PHI <D.20903_14(ab)(18)>
# D.20903_629(ab) = PHI <D.20903_14(ab)(36)>
and then one:
# D.20903_630(ab) = PHI <D.20903_14(ab)(27), D.20903_628(ab)(25), D.20903_629(ab)(39), D.20903_14(ab)(42), D.20903_14(ab)(4), D
.20903_627(ab)(8), D.20903_14(ab)(11)>
the 3 are only used in this last PHI, so I wonder what was the point creating them.
D.20903_630 is then used couple of times and so is the original D.20903_14.
Comment 2 Richard Biener 2008-02-12 22:03:57 UTC
Actually it is the call to rewrite_into_loop_closed_ssa inserting these
PHIs.  I don't know if it actually makes sense to speak of loop-closed SSA form
if we deal with abnormal SSA names - Zdenek?
Comment 3 Richard Biener 2008-02-12 22:45:40 UTC
Reduced testcase without include:

typedef unsigned int size_t;
template<typename _Iterator, typename _Container> class __normal_iterator {
public:
  const _Iterator& base() const;
};
template<typename _BI1, typename _BI2> inline
void copy_backward(_BI1 __first, _BI1 __last, _BI2 __result) {
  while (__first != __last) *--__result = *--__last;
}
template<typename _Tp> struct _Vector_base {
  struct _Vector_impl { _Tp* _M_finish; };
  _Vector_impl _M_impl;
};
template<typename _Tp > class vector : protected _Vector_base<_Tp> {
  typedef vector<_Tp> vector_type;
  typedef _Tp * pointer;
  typedef _Tp & reference;
  typedef __normal_iterator<pointer, vector_type> iterator;
  typedef size_t size_type;
public:
  iterator end();
  void resize(size_type __new_size) { insert(end(), __new_size); }
  reference operator[](size_type __n);
  void insert(iterator __position, size_type __n)
  {
    pointer __old_finish(this->_M_impl._M_finish);
    copy_backward(__position.base(), __old_finish - __n, __old_finish);
  }
};
struct A {
  virtual ~A ();
  void incRef ();
  void decRef ();
};
struct C : public A {
  static C *alloc ();
};
template <class T> struct B {
  B () : ptr (T::alloc ()) { }
  B (T *a_ptr) : ptr (a_ptr) { }
  ~B () { decRef (); }
  B& operator= (const B<T>& a) { if (a.get () != this->get ()) { decRef (); incRef (); } }
  template<class U> operator B<U> () const { return B<U> (ptr); }
  T* operator-> () const { }
  T* get () const { return ptr; }
  void decRef () const { if (ptr != 0) ptr->decRef (); }
  void incRef () const { if (ptr != 0) ptr->incRef (); }
  T *ptr;
};
struct D : public C {
  template <class T> inline void foo (const B<T> & x) { d.resize (1); d[0] = x; }
  vector<B <C> > d;
};
struct E : public C {
  static E *alloc ();
};
struct F : public D {
  static F *alloc ();
};
void foo (vector<B<D> > & x) {
  for (int i = 0; i < 2; ++i)
    {
      B<F> l;
      B<E> m;
      l->foo (m);
    }
}
Comment 4 rakdver@kam.mff.cuni.cz 2008-02-13 04:05:15 UTC
Subject: Re:  [4.3 regression] Unable to coalesce ab SSA_NAMEs

> Actually it is the call to rewrite_into_loop_closed_ssa inserting these
> PHIs.  I don't know if it actually makes sense to speak of loop-closed SSA form
> if we deal with abnormal SSA names - Zdenek?

Some pieces of the scev analysis assume lcSSA for all gimple_reg SSA
names, regardless of whether they appear in abnormal phi nodes or not.
The phi nodes created by lcSSA conversion should always be eliminable,
though.
Comment 5 Zdenek Dvorak 2008-02-15 02:59:13 UTC
forwprop3 changes

SR.40_22 = &D.2672_11(ab)->D.2242;
# D.2672_315(ab) = PHI <D.2672_11(ab)(14), D.2672_11(ab)(19), D.2672_11(ab)(17)>
SR.40_27 = SR.40_22;
D.2705_29 = &SR.40_27->D.2120;

(where the life ranges of D_11 and D_315 do not overlap)

to

SR.40_22 = &D.2672_11(ab)->D.2242;
# D.2672_315(ab) = PHI <D.2672_11(ab)(14), D.2672_11(ab)(19), D.2672_11(ab)(17)>
SR.40_27 = SR.40_22;
D.2705_29 = &D.2672_11(ab)->D.2242.D.2120;

(where the life ranges of D_11 and D_315 do overlap).
Fwprop probably misses the check for abnormal ssa names, somewhere.
Comment 6 Richard Biener 2008-02-15 09:23:24 UTC
Yeah, forwprop checks that it may propagate the name SR.40_22, but it doesn't
check recursively if any of the names occuring in the ADDR_EXPR of the rhs
are marked abnormal.

We should check for this before calling forward_propagate_addr_expr in
tree_ssa_forward_propagate_single_use_vars.  And probably add a new predicate
tree_contains_abnormal_ssa_name_p (tree exp) which walks exp recursively
(consider &b->c[d(AB)] or as in this case &a(AB)->x...).

Please unassign yourself if you want me to do it.
Comment 7 Richard Biener 2008-02-15 12:36:17 UTC
*** Bug 35182 has been marked as a duplicate of this bug. ***
Comment 8 Richard Biener 2008-02-15 12:37:07 UTC
With confirming that PR35182 is the same issue as this I produced a patch.
Comment 9 rakdver@kam.mff.cuni.cz 2008-02-15 14:06:02 UTC
Subject: Re:  [4.3 regression] Unable to coalesce ab SSA_NAMEs

> Yeah, forwprop checks that it may propagate the name SR.40_22, but it doesn't
> check recursively if any of the names occuring in the ADDR_EXPR of the rhs
> are marked abnormal.
> 
> We should check for this before calling forward_propagate_addr_expr in
> tree_ssa_forward_propagate_single_use_vars.  And probably add a new predicate
> tree_contains_abnormal_ssa_name_p (tree exp) which walks exp recursively
> (consider &b->c[d(AB)] or as in this case &a(AB)->x...).
> 
> Please unassign yourself if you want me to do it.

actually, I have a patch in testing.
Comment 10 rguenther@suse.de 2008-02-15 14:12:25 UTC
Subject: Re:  [4.3 regression] Unable to coalesce
 ab SSA_NAMEs

On Fri, 15 Feb 2008, rakdver at kam dot mff dot cuni dot cz wrote:

> ------- Comment #9 from rakdver at kam dot mff dot cuni dot cz  2008-02-15 14:06 -------
> Subject: Re:  [4.3 regression] Unable to coalesce ab SSA_NAMEs
> 
> > Yeah, forwprop checks that it may propagate the name SR.40_22, but it doesn't
> > check recursively if any of the names occuring in the ADDR_EXPR of the rhs
> > are marked abnormal.
> > 
> > We should check for this before calling forward_propagate_addr_expr in
> > tree_ssa_forward_propagate_single_use_vars.  And probably add a new predicate
> > tree_contains_abnormal_ssa_name_p (tree exp) which walks exp recursively
> > (consider &b->c[d(AB)] or as in this case &a(AB)->x...).
> > 
> > Please unassign yourself if you want me to do it.
> 
> actually, I have a patch in testing.

Ok.  I also have finished testing a patch.  Let's just pick the nicer one,
so just post yours after it finished testing.

Richard.
Comment 11 Richard Biener 2008-02-15 15:25:07 UTC
Subject: Bug 35164

Author: rguenth
Date: Fri Feb 15 15:24:19 2008
New Revision: 132345

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132345
Log:
2008-02-15  Richard Guenther  <rguenther@suse.de>
	Zdenek Dvorak  <ook@ucw.cz>

	PR tree-optimization/35164
	* tree-flow.h (stmt_references_abnormal_ssa_name): Declare.
	* tree-dfa.c (stmt_references_abnormal_ssa_name): New function.
	* tree-ssa-forwprop.c (tree_ssa_forward_propagate_single_use_vars):
	Only propagate addresses which do not have abnormal SSA_NAMEs
	in their operands.

	* g++.dg/torture/pr35164-1.C: New testcase.
	* g++.dg/torture/pr35164-2.C: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/torture/pr35164-1.C
    trunk/gcc/testsuite/g++.dg/torture/pr35164-2.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-dfa.c
    trunk/gcc/tree-flow.h
    trunk/gcc/tree-ssa-forwprop.c

Comment 12 Richard Biener 2008-02-15 15:25:43 UTC
Fixed.  Thanks Zdenek for the initial analysis.
Comment 13 Richard Biener 2020-07-28 05:54:58 UTC
*** Bug 30604 has been marked as a duplicate of this bug. ***