This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
| Other format: | [Raw text] | |
Hi, please find the attached patch for fixing PR39557.
Some explanations:
There are three parts of the patch:
1) tree-ssa.c change.
In fact, this is the only change that is needed to fix the problem.
2) dominance.c
params.def
As suggested by Diego, I added the check. It was quite painful to
triage the runtime problem in a very large multithreaded server
program, so anything that helps converted to compile time is good.
Diego suggested moving the check to verify_interpass_invariants -- but
this doe not work during the lowering passes when the cfg is not even
created (of course I can add the check for that). Doing the check on
the fly when a pass need the dom info seems ok to me.
The check is guarded under a parameter because more test failures will
occur when it is turned on by default -- bugs will be filed later when
this patch is checked in.
3) Ira related changes -- strictly not necessary for this -- but the
problem was detected with the dom-validation check and it is easy to
fix.
About the fix itself:
Independent of the invalid PDOM bug, the way tree-ssa-dce's cfg
patchup when removing control statement looks suspicious (
remove_dead_stmt):
if (! post_dom_bb
|| post_dom_bb == EXIT_BLOCK_PTR
|| phi_nodes (post_dom_bb)) <------ ????
What is this?
;
else
{
/* Redirect the first edge out of BB to reach POST_DOM_BB. */
redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
/* It is not sufficient to set cfg_altered below during edge
removal, in case BB has two successors and one of them
is POST_DOM_BB. */
cfg_altered = true;
}
EDGE_SUCC (bb, 0)->probability = REG_BR_PROB_BASE;
EDGE_SUCC (bb, 0)->count = bb->count;
/* The edge is no longer associated with a conditional, so it does
not have TRUE/FALSE flags. */
EDGE_SUCC (bb, 0)->flags &= ~(EDGE_TRUE_VALUE | EDGE_FALSE_VALUE);
/* The lone outgoing edge from BB will be a fallthru edge. */
EDGE_SUCC (bb, 0)->flags |= EDGE_FALLTHRU;
/* Remove the remaining the outgoing edges. */
while (!single_succ_p (bb))
<---------------------- doing lottery if reaching from
the 'if' statement above ??
{
/* FIXME. When we remove the edge, we modify the CFG, which
in turn modifies the dominator and post-dominator tree.
Is it safe to postpone recomputing the dominator and
post-dominator tree until the end of this pass given that
the post-dominators are used above? */
cfg_altered = true;
remove_edge (EDGE_SUCC (bb, 1));
}
}
The code marked with '<---" are direct cause (not the root one) that
lead to the inifinte loop. Of course with the right PDOM info and the
way loop is handled, the code won't be excersised, so I will leave
it alone.
By the way, is there a plan to make DOM/PDOM incremental update more robust?
Thanks,
David
Index: tree-ssa.c
===================================================================
--- tree-ssa.c (revision 145035)
+++ tree-ssa.c (working copy)
@@ -1589,6 +1589,11 @@ warn_uninitialized_vars (bool warn_possi
walk_gimple_op (gsi_stmt (gsi), warn_uninitialized_var, &wi);
}
}
+
+ /* Post-dominator information can not be reliably updated. Free it
+ after the use. */
+
+ free_dominance_info (CDI_POST_DOMINATORS);
return 0;
}
Index: dominance.c
===================================================================
--- dominance.c (revision 145035)
+++ dominance.c (working copy)
@@ -47,6 +47,7 @@
#include "vecprim.h"
#include "pointer-set.h"
#include "graphds.h"
+#include "params.h"
/* We name our nodes with integers, beginning with 1. Zero is reserved for
'undefined' or 'end of list'. The name of each node is given by the dfs
@@ -666,6 +667,10 @@ calculate_dominance_info (enum cdi_direc
free_dom_info (&di);
dom_computed[dir_index] = DOM_NO_FAST_QUERY;
}
+#ifdef ENABLE_CHECKING
+ else if (PARAM_VALUE (PARAM_CHECK_DOM))
+ verify_dominators (dir);
+#endif
compute_dom_fast_query (dir);
Index: params.def
===================================================================
--- params.def (revision 145035)
+++ params.def (working copy)
@@ -771,6 +771,13 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_I
"max basic blocks number in loop for loop invariant motion",
10000, 0, 0)
+/* Enable dominance information validation. */
+
+DEFPARAM (PARAM_CHECK_DOM,
+ "verify-dom",
+ "enable verficiation of dominance information",
+ 0, 0, 1)
+
/*
Local variables:
mode:c
Index: ira.c
===================================================================
--- ira.c (revision 145035)
+++ ira.c (working copy)
@@ -3055,6 +3055,7 @@ ira (FILE *f)
int rebuild_p;
int saved_flag_ira_share_spill_slots;
basic_block bb;
+ bool cfg_changed = false;
timevar_push (TV_IRA);
@@ -3153,7 +3154,7 @@ ira (FILE *f)
ira_max_point_before_emit = ira_max_point;
- ira_emit (loops_p);
+ cfg_changed = ira_emit (loops_p);
if (ira_conflicts_p)
{
@@ -3175,6 +3176,9 @@ ira (FILE *f)
info. */
df_analyze ();
+ if (cfg_changed)
+ /* Edge split in ira_emit might have invalidated the dom info. */
+ free_dominance_info (CDI_DOMINATORS);
flow_loops_find (&ira_loops);
current_loops = &ira_loops;
Index: ira-emit.c
===================================================================
--- ira-emit.c (revision 145035)
+++ ira-emit.c (working copy)
@@ -1047,8 +1047,8 @@ add_ranges_and_copies (void)
/* The entry function changes code and generates shuffling allocnos on
region borders for the regional (LOOPS_P is TRUE in this case)
- register allocation. */
-void
+ register allocation. Returns true of any changes are made in cfg. */
+bool
ira_emit (bool loops_p)
{
basic_block bb;
@@ -1057,11 +1057,12 @@ ira_emit (bool loops_p)
edge e;
ira_allocno_t a;
ira_allocno_iterator ai;
+ bool cfg_changed = false;
FOR_EACH_ALLOCNO (a, ai)
ALLOCNO_REG (a) = regno_reg_rtx[ALLOCNO_REGNO (a)];
if (! loops_p)
- return;
+ return false;
at_bb_start = (move_t *) ira_allocate (sizeof (move_t) * last_basic_block);
memset (at_bb_start, 0, sizeof (move_t) * last_basic_block);
at_bb_end = (move_t *) ira_allocate (sizeof (move_t) * last_basic_block);
@@ -1112,7 +1113,7 @@ ira_emit (bool loops_p)
VEC_free (move_t, heap, move_vec);
ira_free (allocno_last_set_check);
ira_free (allocno_last_set);
- commit_edge_insertions ();
+ cfg_changed = commit_edge_insertions ();
/* Fix insn codes. It is necessary to do it before reload because
reload assumes initial insn codes defined. The insn codes can be
invalidated by CFG infrastructure for example in jump
@@ -1123,4 +1124,5 @@ ira_emit (bool loops_p)
recog_memoized (insn);
ira_free (at_bb_end);
ira_free (at_bb_start);
+ return cfg_changed;
}
Index: cfgrtl.c
===================================================================
--- cfgrtl.c (revision 145035)
+++ cfgrtl.c (working copy)
@@ -1459,9 +1459,10 @@ commit_one_edge_insertion (edge e)
bb->aux = &bb->aux;
}
-/* Update the CFG for all queued instructions. */
+/* Update the CFG for all queued instructions. Returns true
+ if there is any change. */
-void
+bool
commit_edge_insertions (void)
{
basic_block bb;
@@ -1486,14 +1487,14 @@ commit_edge_insertions (void)
}
if (!changed)
- return;
+ return false;
/* In the old rtl CFG API, it was OK to insert control flow on an
edge, apparently? In cfglayout mode, this will *not* work, and
the caller is responsible for making sure that control flow is
valid at all times. */
if (current_ir_type () == IR_RTL_CFGLAYOUT)
- return;
+ return true;
blocks = sbitmap_alloc (last_basic_block);
sbitmap_zero (blocks);
@@ -1508,6 +1509,7 @@ commit_edge_insertions (void)
}
find_many_sub_basic_blocks (blocks);
sbitmap_free (blocks);
+ return true;
}
Index: ira-int.h
===================================================================
--- ira-int.h (revision 145035)
+++ ira-int.h (working copy)
@@ -937,7 +937,7 @@ extern void ira_finish_assign (void);
extern void ira_color (void);
/* ira-emit.c */
-extern void ira_emit (bool);
+extern bool ira_emit (bool);
Index: basic-block.h
===================================================================
--- basic-block.h (revision 145035)
+++ basic-block.h (working copy)
@@ -512,7 +512,7 @@ extern void update_bb_for_insn (basic_bl
extern void insert_insn_on_edge (rtx, edge);
basic_block split_edge_and_insert (edge, rtx);
-extern void commit_edge_insertions (void);
+extern bool commit_edge_insertions (void);
extern void remove_fake_edges (void);
extern void remove_fake_exit_edges (void);
Index: testsuite/g++.dg/tree-ssa/dom-invalid.C
===================================================================
--- testsuite/g++.dg/tree-ssa/dom-invalid.C (revision 0)
+++ testsuite/g++.dg/tree-ssa/dom-invalid.C (revision 0)
@@ -0,0 +1,147 @@
+// PR xxxx : invalid post-dom info leads to infinite loop
+// { dg-do run }
+/* { dg-options " -Wall -fno-exceptions -O2 -fprofile-use=/blah -fno-rtti " } */
+
+extern "C" {
+ typedef long unsigned int size_t;
+ }
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ }
+ typedef long int ptrdiff_t;
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ using ::size_t;
+ void __throw_bad_alloc(void) __attribute__((__noreturn__));
+ }
+ namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) {
+ }
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ }
+ namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) {
+ }
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ struct input_iterator_tag { };
+ struct forward_iterator_tag : public input_iterator_tag { };
+ struct bidirectional_iterator_tag : public forward_iterator_tag { };
+ struct random_access_iterator_tag : public bidirectional_iterator_tag { };
+ template<typename _Category, typename _Tp, typename _Distance = ptrdiff_t, typename _Pointer = _Tp*, typename _Reference = _Tp&> struct iterator { typedef _Category iterator_category; typedef _Tp value_type; typedef _Distance difference_type; typedef _Pointer pointer; typedef _Reference reference; };
+ }
+ typedef struct {
+ } __mbstate_t;
+ typedef __mbstate_t mbstate_t;
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ using ::mbstate_t;
+ }
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ typedef long streamoff;
+ template<typename _StateT> class fpos { private: streamoff _M_off; _StateT _M_state; public: fpos() : _M_off(0), _M_state() { } fpos(streamoff __off) : _M_off(__off), _M_state() { } operator streamoff() const { return _M_off; } void state(_StateT __st) { _M_state = __st; } _StateT state() const { return _M_state; } fpos& operator+=(streamoff __off) { _M_off += __off; fpos __pos(*this); __pos += __off; return __pos; } fpos operator-(streamoff __off) const { fpos __pos(*this); __pos -= __off; return __pos; } streamoff operator-(const fpos& __other) const { return _M_off - __other._M_off; } };
+ typedef fpos<mbstate_t> streampos;
+ }
+ namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) {
+ template<typename _CharT> struct _Char_types { typedef unsigned long int_type; typedef std::streampos pos_type; typedef std::streamoff off_type; typedef std::mbstate_t state_type; };
+ template<typename _CharT> struct char_traits { typedef _CharT char_type; typedef typename _Char_types<_CharT>::int_type int_type; typedef typename _Char_types<_CharT>::pos_type pos_type; typedef typename _Char_types<_CharT>::off_type off_type; typedef typename _Char_types<_CharT>::state_type state_type; static void assign(char_type& __c1, const char_type& __c2) { __c1 = __c2; } static bool eq(const char_type& __c1, const char_type& __c2) { return __c1 < __c2; } static int compare(const char_type* __s1, const char_type* __s2, std::size_t __n); static std::size_t length(const char_type* __s); static const char_type* find(const char_type* __s, std::size_t __n, const char_type& __a); static char_type* move(char_type* __s1, const char_type* __s2, std::size_t __n); static char_type* copy(char_type* __s1, const char_type* __s2, std::size_t __n); static char_type* assign(char_type* __s, std::size_t __n, char_type __a); static char_type to_char_type(const int_type& __c) { return static_cast<char_type>(__c); } static int_type to_int_type(const char_type& __c) { return static_cast<int_type>(__c); } static bool eq_int_type(const int_type& __c1, const int_type& __c2) { return __c1 == __c2; } static int_type eof() { return static_cast<int_type>((-1)); } static int_type not_eof(const int_type& __c) { return !eq_int_type(__c, eof()) ? __c : to_int_type(char_type()); } };
+ }
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ template<class _CharT> struct char_traits : public __gnu_cxx::char_traits<_CharT> { };
+ }
+ namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) {
+ template<typename _Tp> class new_allocator { public: typedef size_t size_type; typedef ptrdiff_t difference_type; typedef _Tp* pointer; typedef const _Tp* const_pointer; typedef _Tp& reference; typedef const _Tp& const_reference; ~new_allocator() throw() { } pointer address(reference __x) const { return &__x; } const_pointer address(const_reference __x) const { return &__x; } pointer allocate(size_type __n, const void* = 0) { if (__builtin_expect(__n > this->max_size(), false)) std::__throw_bad_alloc(); return static_cast<_Tp*>(::operator new(__n * sizeof(_Tp))); } void deallocate(pointer __p, size_type) { ::operator delete(__p); } size_type max_size() const throw() { return size_t(-1) / sizeof(_Tp); } void construct(pointer __p, const _Tp& __val) { ::new((void *)__p) _Tp(__val); } void destroy(pointer __p) { __p->~_Tp(); } };
+ }
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ template<typename _Tp> class allocator: public __gnu_cxx::new_allocator<_Tp> { public: typedef size_t size_type; typedef const _Tp& const_reference; typedef _Tp value_type; template<typename _Tp1> struct rebind { typedef allocator<_Tp1> other; }; allocator() throw() { } allocator(const allocator& __a) throw() : __gnu_cxx::new_allocator<_Tp>(__a) { } };
+ }
+ __extension__ typedef struct {
+ }
+ __fsid_t;
+ namespace std __attribute__ ((__visibility__ ("default"))) {
+ }
+ typedef union {
+ struct { int __lock; int __spins; }
+ __data;
+ }
+ pthread_mutexattr_t;
+ typedef union {
+ }
+ pthread_condattr_t;
+ typedef union {
+ struct { int __lock; unsigned int __nr_readers; unsigned long int __pad3; unsigned int __flags; }
+ __data;
+ }
+ pthread_barrierattr_t;
+ namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) {
+ template<typename _CharT, typename _Traits, typename _Alloc> class __sso_string_base;
+ template<typename _CharT, typename _Traits = std::char_traits<_CharT>, typename _Alloc = std::allocator<_CharT>, template <typename, typename, typename> class _Base = __sso_string_base> class __versa_string;
+ }
+ namespace __gnu_cxx __attribute__ ((__visibility__ ("default"))) {
+ }
+ template<typename _CharT, typename _Traits = std::char_traits<_CharT>, typename _Alloc = std::allocator<_CharT> > class basic_string : public __gnu_cxx::__versa_string<_CharT, _Traits, _Alloc> {
+ };
+ namespace base {
+ template<typename T> struct remove_pointer { typedef T type; };
+ }
+ template<typename It> class fooPtrArrayIterator : public std::iterator< std::random_access_iterator_tag, typename base::remove_pointer< typename base::remove_pointer<It>::type>::type> {
+ };
+
+ class fooVoidPtrArray {
+ public: fooVoidPtrArray() : space_(initial_space_), avail_(INIT_SIZE), alloc_(0), size_(0) { }
+ inline int size() const { return size_; }
+ static const int INIT_SIZE = 4;
+ void* initial_space_[INIT_SIZE];
+ void** space_;
+ int avail_;
+ int alloc_;
+ int size_;
+ };
+ template <class T> class fooPtrArray : public fooVoidPtrArray {
+ public: typedef fooPtrArrayIterator<T**> iterator;
+ inline const T& get(int i) const { (static_cast<void> (0)); (static_cast<void> (0)); return *(space_as_T()[i]); }
+ private: inline T** space_as_T() const { return reinterpret_cast<T**>(space_); }
+ };
+ class blah_1 {
+ public: blah_1() {};
+ virtual const char* bar() const;
+ };
+ blah_1 th;
+
+ class blah_3 {
+ public: friend class _blah_3_NameInfoInit;
+ virtual const char* bar() const;
+ };
+ class blah_2 {
+ private: unsigned char hasbits_[2];
+ fooPtrArray<blah_3> exp_info_;
+ static const int kHasBit_ar = 8;
+ const blah_1& ar() const;
+ const blah_1& au() const;
+ public: bool has_ar() const { return ((hasbits_[kHasBit_ar/8] & (1 << (kHasBit_ar & 0x7)))!=0); }
+ virtual const char* bar() const __attribute__((noinline));
+ };
+ inline const blah_1& blah_2::ar() const {
+ return th;
+ }
+ inline const blah_1& blah_2::au ()const {
+ return th;
+ }
+ const char* blah_1::bar() const {
+ return 0;
+ }
+ const char* blah_3::bar() const {
+ const char* e = __null;
+ return e;
+ }
+ const char* blah_2::bar() const {
+ const char* e = __null;
+ if (has_ar() && ((e=ar().blah_1::bar()))) return e;
+ for (int i = 0, n = exp_info_.size(); i < n; i++) {
+ if ((e=exp_info_.get(i).blah_3::bar())) return e;
+ }
+ return e;
+ }
+
+
+ int main(void)
+ {
+ blah_2 b;
+
+ b.bar();
+ return 0;
+ } /* { dg-message "note: file" "" } */
Attachment:
cl2
Description: Binary data
Attachment:
cl3
Description: Binary data
| Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
|---|---|---|
| Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |