Bug 85929 - _GLIBCXX_ASSERTIONS, subscript type mismatch, and std::vector bounds check elimination
Summary: _GLIBCXX_ASSERTIONS, subscript type mismatch, and std::vector bounds check el...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks: VRP std::vector
  Show dependency treegraph
 
Reported: 2018-05-25 19:40 UTC by Florian Weimer
Modified: 2024-12-21 08:58 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2024-12-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2018-05-25 19:40:47 UTC
Compile this with “gcc -O2” on a 64-bit platform:

#define _GLIBCXX_ASSERTIONS
#include <vector>

typedef unsigned int size_type;

void
increment (std::vector<double> &vec)
{
   size_type count = vec.size();
   for (size_type i = 0; i < count; ++i)
     vec[i] += 1;
}

The expectation is that there is no call to abort.  The bounds check is optimized away if the (global) size_type is itself size_t/unsigned long (matching the index type of the vector).

A loop like this appears in the dealII benchmark from SPEC2006, and it is a major source of performance regression with _GLIBCXX_ASSERTIONS.
Comment 1 Marc Glisse 2018-05-25 20:51:50 UTC
With size_type = unsigned long, the bounds check turns out to be exactly the same test as the loop exit check, and FRE3 gets rid of it.

With size_type = unsigned int, it is harder. We have roughly

  long int _16;
  unsigned int i;
  count_8 = (unsigned int) _16;
  if (i_4 >= count_8)
[... else branch]
  _1 = (long unsigned int) i_4;
  _21 = (long unsigned int) _16;
  if (_1 >= _21)

It is true that i < (unsigned)_16 implies (unsigned long)i < (unsigned long)_16, though that's clearly not an equivalence. So the transformation should be possible, but I am not sure how to fit it into FRE (or anywhere).
Comment 2 Richard Biener 2018-05-28 08:17:11 UTC
VRP sees

  long int _16;
  size_type count;

  count_8 = (size_type) _16;
...
  <bb 3> [local count: 593525634]:
  # i_4 = PHI <0(2), i_12(6)>
  if (i_4 >= count_8)
    goto <bb 7>; [11.00%]
  else
    goto <bb 4>; [89.00%]

  <bb 4> [local count: 528237814]:
  i_18 = ASSERT_EXPR <i_4, i_4 < count_8>;
  _1 = (long unsigned int) i_18;
  _21 = (long unsigned int) _16;
...
  if (_1 >= _21)

so we could improve things in vrp_evaluate_conditional and friends by looking
not only at the ops but their definitions.  But the truncation of _16 to count_8 makes the desired optimization more complicated...  That is,
for > UINT_MAX # of elements the code will infintely loop AFAICS (but it will
not access elements out of bounds).

So somehow we need to enhance the code in VRP that registers additional
asserts to also handle symbolic ranges and thus register not only
i_4 < count_8 but also (long int) i_4 < _16 in a usable form.
Comment 3 Florian Weimer 2018-05-28 09:26:24 UTC
(In reply to Richard Biener from comment #2)
> That is,
> for > UINT_MAX # of elements the code will infintely loop AFAICS (but it will
> not access elements out of bounds).

The way I read the original source code, the code will simply not touch all the vector elements in that case (but there is still no out-of-bounds access).
Comment 4 Marc Glisse 2018-05-28 15:29:13 UTC
(In reply to Richard Biener from comment #2)
> So somehow we need to enhance the code in VRP that registers additional
> asserts to also handle symbolic ranges and thus register not only
> i_4 < count_8 but also (long int) i_4 < _16 in a usable form.

(long int) i_4 < _16 may not hold, _16 could easily be negative.