Re: [PATCH 2/2] ifcvt: unfixed testcase for 2nd issue within PR tree-optimization/84178

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Mar 7, 2018 at 7:56 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> Here's a testcase for the 2nd issue within PR tree-optimization/84178.
>
> With -03 -fno-tree-forwprop, trunk and gcc 7 segfault inside update_ssa
> when called from version_loop_for_if_conversion when a loop is versioned.
>
> During loop_version, some blocks are duplicated, and this can duplicate
> SSA names, leading to the duplicated SSA names being added to
> tree-into-ssa.c's old_ssa_names.
>
> For example, _117 is an SSA name defined in one of these to-be-duplicated
> blocks, and hence is added to old_ssa_names when duplicated.
>
> One of the BBs has this gimplified predicate (again, these stmts are *not*
> yet in a BB):
>   _173 = h4.1_112 != 0;
>   _171 = (signed char) _117;
>   _172 = _171 >= 0;
>   _170 = ~_172;
>   _169 = _170 & _173;
>
> Note the reference to SSA name _117 in the above.
>
> When update_ssa runs later on in version_loop_for_if_conversion,
> SSA name _117 is in the old_ssa_names bitmap, and thus has
> prepare_use_sites_for called on it:
>
> 2771      /* If an old name is in NAMES_TO_RELEASE, we cannot remove it from
> 2772         OLD_SSA_NAMES, but we have to ignore its definition site.  */
> 2773      EXECUTE_IF_SET_IN_BITMAP (old_ssa_names, 0, i, sbi)
> 2774        {
> 2775          if (names_to_release == NULL || !bitmap_bit_p (names_to_release, i))
> 2776            prepare_def_site_for (ssa_name (i), insert_phi_p);
> 2777          prepare_use_sites_for (ssa_name (i), insert_phi_p);
> 2778        }
>
> prepare_use_sites_for iterates over the immediate users, which includes
> the:
>   _171 = (signed char) _117;
> in the gimplified predicate.
>
> This tried to call "mark_block_for_update" on the BB of this def_stmt,
> leading to a read-through-NULL segfault, since this statement isn't in a
> BB yet.
>
> In an earlier attempt to fix this I wrote:
>> The following patch fixes the 2nd ICE by inserting the gimplified predicates
>> earlier: immediately before the possible version_loop_for_if_conversion,
>> rather than within combine_blocks.  That way they're in their BBs before
>> we attempt any further manipulation of the CFG.
>
> to which Richard responded:
>> Hum, but that will alter both copies of the loops, no?  I think the fix is
>> to instead delay the update_ssa call to _after_ combine_blocks ()
>> (and remember if it is necessary just in case we didn't version).
>
> I attempted this, but didn't come up with something that worked; so am
> posting this in the hope that Richard (or someone else) can finish the
> fix.

Preliminary testing shows the attached works.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2018-03-08  Richard Biener  <rguenther@suse.de>

        PR tree-optimization/84178
        * tree-if-conv.c (combine_blocks): Move insert_gimplified_predicates
        to caller.
        (version_loop_for_if_conversion): Delay update_ssa call.
        (tree_if_conversion): Delay update_ssa until after predicate
        insertion.

        * gcc.dg/torture/pr84178-2.c: New testcase.


> gcc/testsuite/ChangeLog:
>         PR tree-optimization/84178
>         * gcc.c-torture/compile/pr84178-2.c: New test.
> ---
>  gcc/testsuite/gcc.c-torture/compile/pr84178-2.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr84178-2.c
>
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr84178-2.c b/gcc/testsuite/gcc.c-torture/compile/pr84178-2.c
> new file mode 100644
> index 0000000..b2548e9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr84178-2.c
> @@ -0,0 +1,18 @@
> +/* { dg-options "-fno-tree-forwprop" } */
> +
> +int zy, h4;
> +
> +void
> +r8 (long int mu, int *jr, int *fi, short int dv)
> +{
> +  do
> +    {
> +      int tx;
> +
> +      tx = !!h4 ? (zy + h4) : 1;
> +      mu = tx;
> +      *jr = (((unsigned char) mu > (254 >> dv)) ? 0 : (unsigned char) tx) + *fi;
> +    } while (*jr == 0);
> +
> +  r8 (mu, jr, fi, 1);
> +}
> --
> 1.8.5.3
>

Attachment: fix-pr84178
Description: Binary data