[Bug tree-optimization/104196] [12 Regression] wrong code at -O2 and above on x86_64-linux-gnu

["cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104196

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:82c8ff79d06cc7d389c72f94d4443c509cf85313

commit r12-6888-g82c8ff79d06cc7d389c72f94d4443c509cf85313
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Jan 27 10:47:00 2022 +0100

    reassoc: Fix up inter-bb range optimization [PR104196]

    As mentioned in the PR, reassoc1 miscompiles following testcase.
    We have:
      if (a.1_1 >= 0)
        goto <bb 5>; [59.00%]
      else
        goto <bb 4>; [41.00%]

      <bb 4> [local count: 440234144]:
      _3 = -2147483647 - a.1_1;
      _9 = a.1_1 != -2147479551;
      _4 = _3 == 1;
      _8 = _4 | _9;
      if (_8 != 0)
        goto <bb 5>; [34.51%]
      else
        goto <bb 3>; [65.49%]

    and the inter-bb range test optimization treats it as:
      if ((a.1_1 >= 0)
          | (-2147483647 - a.1_1 == 1)
          | (a.1_1 != -2147479551))
        goto bb5;
      else
        goto bb3;
    and recognizes that a.1_1 >= 0 is redundant with a.1_1 != -2147479551
    and so will optimize it into:
      if (0
          | (-2147483647 - a.1_1 == 1)
          | (a.1_1 != -2147479551))
        goto bb5;
      else
        goto bb3;
    When merging 2 comparisons, we use update_range_test which picks one
    of the comparisons as the one holding the result (currently always
    the RANGE one rather than all the OTHERRANGE* ones) and adjusts the
    others to be true or false.
    The problem with doing that is that means the
      _3 = -2147483647 - a.1_1;
    stmt with undefined behavior on overflow used to be conditional before
    but now is unconditional.  reassoc performs a no_side_effect_bb check
    which among other checks punts on gimple_has_side_effects and
    gimple_assign_rhs_could_trap_p stmts as well as ones that have uses of
    their lhs outside of the same bb, but it doesn't punt for this potential
    signed overflow case.

    Now, for this testcase, it can be fixed in update_range_test by being
    smarter and choosing the other comparison to modify.  This is achieved
    by storing into oe->id index of the bb with GIMPLE_COND the
    comparison feeds into not just for the cases where the comparison is
    the GIMPLE_COND itself, but in all cases, and then finding oe->id that
    isn't dominated by others.  If we find such, use that oe for the merge
    test and if successful, swap range->idx and swap_with->idx.
    So for the above case we optimize it into:
      if ((a.1_1 != -2147479551)
          | (-2147483647 - a.1_1 == 1)
          | 0)
        goto bb5;
      else
        goto bb3;
    instead.

    Unfortunately, this doesn't work in all cases,
    optimize_range_tests_to_bit_test and
    optimize_range_tests_cmp_bitwise optimizations use non-NULL seq
    to update_range_test and they carefully choose a particular comparison
    because the sequence then uses SSA_NAMEs that may be defined only in
    their blocks.  For that case, the patch keeps using the chosen comparison
    but if the merge is successful, rewrites stmts with signed overflow
behavior
    into defined overflow.
    For this I ran into a problem, rewrite_to_defined_overflow returns a
    sequence that includes the original stmt with modified arguments, this
means
    it needs to be gsi_remove first.  Unfortunately, gsi_remove regardless of
    the remove_permanently argument creates debug temps for the lhs, which I
    think is quite undesirable here.  So I've added an argument (default to
    false) to rewrite_to_defined_overflow to do the modification in place
    without the need to remove the stmt.

    2022-01-27  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/104196
            * gimple-fold.h (rewrite_to_defined_overflow): Add IN_PLACE
argument.
            * gimple-fold.cc (rewrite_to_defined_overflow): Likewise.  If true,
            return NULL and emit needed stmts before and after stmt.
            * tree-ssa-reassoc.cc (update_range_test): For inter-bb range opt
            pick as operand_entry that will hold the merged test the one
feeding
            earliest condition, ensure that by swapping range->idx with some
            other range's idx if needed.  If seq is non-NULL, don't actually
swap
            it but instead rewrite stmts with undefined overflow in between
            the two locations.
            (maybe_optimize_range_tests): Set ops[]->id to bb->index with the
            corresponding condition even if they have non-NULL ops[]->op.
            Formatting fix.

            * gcc.c-torture/execute/pr104196.c: New test.