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 <rguent...@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;
> }
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham
Norton, HRB 21284 (AG Nuernberg)
