| Message ID | alpine.LSU.2.11.1507101456360.9923@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
On Fri, 10 Jul 2015, Richard Biener wrote: > On Fri, 10 Jul 2015, Richard Biener wrote: > > > > > I was just testing the patch below which runs into latent issues when > > building libjava (at least)... > > > > /space/rguenther/src/svn/trunk/libjava/java/lang/natClassLoader.cc: In > > function ‘java::lang::Class* _Jv_FindClassInCache(_Jv_Utf8Const*)’: > > /space/rguenther/src/svn/trunk/libjava/java/lang/natClassLoader.cc:97:1: > > error:BB 3 last statement has incorrectly set lp > > _Jv_FindClassInCache (_Jv_Utf8Const *name) > > ^ > > /space/rguenther/src/svn/trunk/libjava/java/lang/natClassLoader.cc:97:1: > > internal compiler error: verify_flow_info failed > > 0x8e2132 verify_flow_info() > > /space/rguenther/src/svn/trunk/gcc/cfghooks.c:261 > > > > so I have to debug that first. > > It's stmts no longer throwing after VRP setting a value-range on > an array index for example. I've addressed this in the revised > patch below which teaches CFG cleanup to deal with this (it > already removes dead EH edges and makes similar adjustments for > noreturn calls). > > > Still IMHO the patch makes sense apart > > from the ugly need to go through a INTEGER_CST tree when converting > > a wide_int to a widest_int (ugh). Any wide-int folks around that > > can suggest something better here (reason: the two integers we compare > > do not have to have the same type/precision - see tree_int_cst_lt > > which also uses widest_ints). > > This issue still remains. Fixed with widest_int::from (idx_min, TYPE_SIGN (TREE_TYPE (idx))). But with the patch we run into the general issue that changing a context insensitive predicate to use context sensitive information leads to wrong-code. (Only) gcc.c-torture/execute/pr51933.c fails because if-conversion sees that v2[_18] cannot trap in if (u_14 <= 255) goto <bb 8>; else goto <bb 9>; <bb 8>: # RANGE [0, 255] NONZERO 255 _18 = (int) u_14; _19 = v2[_18]; but of course it uses it to move v2[_18] out of its controling condition. As the patch was supposed to improve if-conversion in the first place (and other passes might be similar confused) I retract it. We'd need a more specialized predicate that also gets context information. Richard. > Bootstrap and regtest running on x86_64-unknown-linux-gnu. > > Richard. > > 2015-07-10 Richard Biener <rguenther@suse.de> > > * tree-eh.c (in_array_bounds_p): Use value-range information > when available. > * tree-cfgcleanup.c (cleanup_control_flow_bb): Clean stmts > from stale EH info. > > Index: gcc/tree-eh.c > =================================================================== > --- gcc/tree-eh.c (revision 225655) > +++ gcc/tree-eh.c (working copy) > @@ -2532,8 +2532,11 @@ in_array_bounds_p (tree ref) > { > tree idx = TREE_OPERAND (ref, 1); > tree min, max; > + wide_int idx_min, idx_max; > > - if (TREE_CODE (idx) != INTEGER_CST) > + if (TREE_CODE (idx) != INTEGER_CST > + && (TREE_CODE (idx) != SSA_NAME > + || get_range_info (idx, &idx_min, &idx_max) != VR_RANGE)) > return false; > > min = array_ref_low_bound (ref); > @@ -2544,11 +2547,26 @@ in_array_bounds_p (tree ref) > || TREE_CODE (max) != INTEGER_CST) > return false; > > - if (tree_int_cst_lt (idx, min) > - || tree_int_cst_lt (max, idx)) > - return false; > + if (TREE_CODE (idx) == INTEGER_CST) > + { > + if (tree_int_cst_lt (idx, min) > + || tree_int_cst_lt (max, idx)) > + return false; > + > + return true; > + } > + else > + { > + if (wi::lts_p (wi::to_widest (wide_int_to_tree (TREE_TYPE (idx), > + idx_min)), > + wi::to_widest (min)) > + || wi::lts_p (wi::to_widest (max), > + wi::to_widest (wide_int_to_tree (TREE_TYPE (idx), > + idx_max)))) > + return false; > > - return true; > + return true; > + } > } > > /* Returns true if it is possible to prove that the range of > Index: gcc/tree-cfgcleanup.c > =================================================================== > --- gcc/tree-cfgcleanup.c (revision 225662) > +++ gcc/tree-cfgcleanup.c (working copy) > @@ -256,6 +256,14 @@ cleanup_control_flow_bb (basic_block bb) > && remove_fallthru_edge (bb->succs)) > retval = true; > > + /* If a stmt may no longer throw, remove it from the EH tables > + and cleanup dead EH edges. */ > + else if (maybe_clean_eh_stmt (stmt)) > + { > + gimple_purge_dead_eh_edges (bb); > + retval = true; > + } > + > return retval; > } >
Index: gcc/tree-eh.c =================================================================== --- gcc/tree-eh.c (revision 225655) +++ gcc/tree-eh.c (working copy) @@ -2532,8 +2532,11 @@ in_array_bounds_p (tree ref) { tree idx = TREE_OPERAND (ref, 1); tree min, max; + wide_int idx_min, idx_max; - if (TREE_CODE (idx) != INTEGER_CST) + if (TREE_CODE (idx) != INTEGER_CST + && (TREE_CODE (idx) != SSA_NAME + || get_range_info (idx, &idx_min, &idx_max) != VR_RANGE)) return false; min = array_ref_low_bound (ref); @@ -2544,11 +2547,26 @@ in_array_bounds_p (tree ref) || TREE_CODE (max) != INTEGER_CST) return false; - if (tree_int_cst_lt (idx, min) - || tree_int_cst_lt (max, idx)) - return false; + if (TREE_CODE (idx) == INTEGER_CST) + { + if (tree_int_cst_lt (idx, min) + || tree_int_cst_lt (max, idx)) + return false; + + return true; + } + else + { + if (wi::lts_p (wi::to_widest (wide_int_to_tree (TREE_TYPE (idx), + idx_min)), + wi::to_widest (min)) + || wi::lts_p (wi::to_widest (max), + wi::to_widest (wide_int_to_tree (TREE_TYPE (idx), + idx_max)))) + return false; - return true; + return true; + } } /* Returns true if it is possible to prove that the range of Index: gcc/tree-cfgcleanup.c =================================================================== --- gcc/tree-cfgcleanup.c (revision 225662) +++ gcc/tree-cfgcleanup.c (working copy) @@ -256,6 +256,14 @@ cleanup_control_flow_bb (basic_block bb) && remove_fallthru_edge (bb->succs)) retval = true; + /* If a stmt may no longer throw, remove it from the EH tables + and cleanup dead EH edges. */ + else if (maybe_clean_eh_stmt (stmt)) + { + gimple_purge_dead_eh_edges (bb); + retval = true; + } + return retval; }