On September 13, 2016 3:32:33 PM GMT+02:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>On Tue, Sep 13, 2016 at 03:21:47PM +0200, Richard Biener wrote:
>> > The following testcase ICEs because SSA_NAME IMM links are broken.
>> > I've tracked it to DOM's optimize_stmt, a GIMPLE_COND in there is
>> > changed, marked as modified, then in optimize_stmt
>> > if (gimple_modified_p (stmt) || modified_p)
>> > {
>> > tree val = NULL;
>> >
>> > update_stmt_if_modified (stmt);
>> > and a few lines later changed again:
>> > if (gimple_code (stmt) == GIMPLE_COND)
>> > {
>> > if (integer_zerop (val))
>> > gimple_cond_make_false (as_a <gcond *> (stmt));
>> > else if (integer_onep (val))
>> > gimple_cond_make_true (as_a <gcond *> (stmt));
>> > without update_stmt. As this is a function which
>update_stmt_if_modified
>> > a few lines before, I think it is fine to update_stmt it
>immediately.
>> >
>> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for
>trunk?
>>
>> Ok if it doesn't do the update twice this way (we do it the "fancy
>way" to
>> only update stmts we modified and do it only once).
>
>It does update it twice this way, the
> update_stmt_if_modified (stmt);
>above is done first and then the newly added update_stmt again.
>
>There is
> if (gimple_code (stmt) == GIMPLE_COND)
> val = fold_binary_loc (gimple_location (stmt),
> gimple_cond_code (stmt), boolean_type_node,
> gimple_cond_lhs (stmt), gimple_cond_rhs (stmt));
> else if (gswitch *swtch_stmt = dyn_cast <gswitch *> (stmt))
> val = gimple_switch_index (swtch_stmt);
>
> if (val && TREE_CODE (val) == INTEGER_CST)
> {
> retval = find_taken_edge (bb, val);
>and gimple_cond_make_{false,true} in between. If none of these care
>about
>the stmt being updated (gimple_cond_make_{false,true} and
>gimple_switch_index is surely ok, dyn_cast too, not 100% sure about
>fold_binary (if it can't use match.pd, though on the other side it is
>not
>passed the stmt, but only its arguments), or find_taken_edge (most
>likely
>ok)), then I could perhaps change the patch to do gimple_set_modified
>(stmt,
>true); where I've added the update_stmt, and move the
>update_stmt_if_modified call after the if (val && TREE_CODE (val) ==
>INTEGER_CST) { ... } stmt.
Yes please, this should be safe. All of the above do not look at immediate
uses but at most use-def links.
Richard.
> Jakub
