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

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

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, to me this doesn't look like a recent reassoc1 regression, but rather
something that been there since
~ r0-119920-gd578e863b08e64789f767c95f85acd10dc477bdf

The inter-bb range optimization pass uses no_side_effect_bb as a test for the
bbs in between, which makes sure that no statements have side-effects, there
are only gimple assignments except for the last GIMPLE_COND and those
statements can't trap and their lhs is consumed within the same bb.

  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%]

is then treated as if ((a.1_1 >= 0) | (-2147483647 - a.1_1 == 1) | (a.1_1 !=
-2147479551)) goto bb5; else goto bb3;
and in there we optimize from there the (a.1_1 >= 0) | (a.1_1 != -2147479551)
part of it into (a.1_1 != -2147479551).  That is all fine, but we have chosen
to put the merged condition into the place of the old _9 test and make the
the a.1_1 >= 0 test (which has been replaced by false).  Usually that doesn't
really matter where we stick it, when merging multiple "ops" into one, one
simply wins (that is the range in update_range_test) and the other(s) are
marked
as true/false (otherrange ... otherrange + count - 1).
The problem above is that the _3 = -2147483647 - a.1_1; stmt is done in
undefined overflow type and thus can trigger UB where there wasn't one before
for certain values.

One possibility to fix this would be to punt on statements like that in
no_side_effect_bb (e.g. the normal way to encode a range is using unsigned
arithmetics).  But I'd be afraid we could easily regress on what the
optimization can handle.

Another possibility is try to be smarter in update_range_test.  Instead of
always picking range->idx as the winner and otherrange*->idx as the stmts
replaced by false or true, perhaps we should try to be smarter, go through them
all first and pick the one from the first bb (one dominating all the other ops)
and if the merge is successful, swap range->idx with the most dominating
otherrange->idx.

So, instead of the problematic
  <bb 2> [local count: 118111598]:

  <bb 3> [local count: 1073741824]:
  a.1_1 = a;
  _3 = -2147483647 - a.1_1;
  _10 = a.1_1 != -2147479551;
  _9 = a.1_1 != -2147479551;
  _4 = _3 == 1;
  _2 = _4 | _10;
  if (_2 != 0)
    goto <bb 4>; [34.51%]
  else
    goto <bb 3>; [65.49%]
we'd get
  a.1_1 = a;
  if (a.1_1 != -2147479551)
    goto <bb 5>; [59.00%]
  else
    goto <bb 4>; [41.00%]

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


Another question but that is GCC 13 material is why we don't optimize
int
foo (int x)
{
  return -2147483647 - x <= 0;
}
into:
  return x >= -2147483647
during GIMPLE (we optimize it during expansion it seems).