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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |glisse at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The intermediate state before it is miscompiled is
_2 = a.0_1 <= 0;
# RANGE [irange] int [0, 1] MASK 0x1 VALUE 0x0
_3 = (int) _2;
_4 = -_3;
# RANGE [irange] int [-INF, +INF] MASK 0xfffffffe VALUE 0x1
_5 = _4 | 1;
# RANGE [irange] int [-INF, +INF] MASK 0xfffffffe VALUE 0x1
b_9 = -_5;
# RANGE [irange] int [-INF, +INF] MASK 0xfffffffe VALUE 0x0
_11 = _3 * -2;
# RANGE [irange] int [-INF, +INF] MASK 0xfffffffe VALUE 0x0
_6 = _11;
# RANGE [irange] int [0, 0][2, 2] MASK 0x2 VALUE 0x0
_7 = _11 & 2;
# RANGE [irange] int [0, 1] MASK 0x1 VALUE 0x0
c_10 = _7 / 2;
if (b_9 != 1)

I think that is fine, _6 (and _11) are still 0 or -2 (the latter for a == 0)
and the ranges seem to be also correct.

I think all the recent uses of with_possible_nonzero_bits2 are wrong and they
all should use with_possible_nonzero_bits instead.

with_possible_nonzero_bits2 has been added for the
/* Slightly extended version, do not make it recursive to keep it cheap.  */
(match (with_possible_nonzero_bits2 @0)
 with_possible_nonzero_bits@0)
(match (with_possible_nonzero_bits2 @0)
 (bit_and:c with_possible_nonzero_bits@0 @2))

/* Same for bits that are known to be set, but we do not have
   an equivalent to get_nonzero_bits yet.  */
(match (with_certain_nonzero_bits2 @0)
 INTEGER_CST@0)
(match (with_certain_nonzero_bits2 @0)
 (bit_ior @1 INTEGER_CST@0))

/* X == C (or X & Z == Y | C) is impossible if ~nonzero(X) & C != 0.  */
(for cmp (eq ne)
 (simplify
  (cmp:c (with_possible_nonzero_bits2 @0) (with_certain_nonzero_bits2 @1))
  (if (wi::bit_and_not (wi::to_wide (@1), get_nonzero_bits (@0)) != 0)
   { constant_boolean_node (cmp == NE_EXPR, type); })))
pattern and I think even for that pattern the uses of those are wrong.
The thing is, when using with_possible_nonzero_bits2 or
with_certain_nonzero_bits2,
if the operand is SSA_NAME with integral/pointer type and its definition is
BIT_AND for the first case and BIT_IOR for the latter case and it matches the
second cases of match, those are preferred and so @0 will be the
BIT_AND/BIT_IOR operand rather than
the whole BIT_AND/BIT_IOR.
Now, if we have say
  # RANGE [irange] int ... MASK 0xb VALUE 0x0
  x_1 = ...;
  # RANGE [irange] int ... MASK 0x8 VALUE 0x0
  _2 = x_1 & 0xc;
  _3 = _2 == 2;
then without the with_possible_nonzero_bits2 we could fold it into _3 = 0;
because get_nonzero_bits (_2) is 8 and 2 & ~8 is 2.
But because with_possible_nonzero_bits2 matched the bit_and form, we won't
optimize it and give up, because get_nonzero_bits (x_1) is 0xb and 2 & ~0xb is
0.
with_certain_nonzero_bits2 is perhaps fine because it matches solely
INTEGER_CST and something | INTEGER_CST.
Though, with current ranger I don't see why with_certain_nonzero_bits2 couldn't
match also SSA_NAME with INTEGER_TYPE_P on which we could do
  if (range_info_p (name) && irange::supports_p (TREE_TYPE (name)))
    {
      int_range_max tmp;
      range_info_get_range (name, tmp);
      irange_bitmask bm = tmp.get_bitmask ();
      return bm.mask () & bm.value ();
    }
As for with_possible_nonzero_bits2, I think we should just remove it.
And, either use some wrapper around get_nonzero_bits or teach get_nonzero_bits
itself to do, if the argument is SSA_NAME with BIT_AND_EXPR definition, also
try to query nonzero_bits on the operands of that (but just once, not walk
further) - perhaps merge the bitmask from those.

Reply via email to