[Bug c++/106921] New: [11/12.1] -O1 and -fipa-icf -fpartial-inlining causes wrong code

[Bug c++/106921] New: [11/12.1] -O1 and -fipa-icf -fpartial-inlining causes wrong code

[lutztonineubert at gmail dot com]

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

            Bug ID: 106921
           Summary: [11/12.1] -O1 and -fipa-icf  -fpartial-inlining causes
                    wrong code
           Product: gcc
           Version: 11.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lutztonineubert at gmail dot com
  Target Milestone: ---

Short summary:

The following code returns 1 if compiled with -O2 (which is wrong) and does
return 0 if compiled without optimization.

```
#include <array>
#include <cstddef>
#include <exception>

#define GCC_VERSION (__GNUC__ * 10000 \
                     + __GNUC_MINOR__ * 100 \
                     + __GNUC_PATCHLEVEL__)
static_assert(GCC_VERSION == 110300);

template <size_t Bits>
class bitset {
 private:
  using word_t = size_t;
  static constexpr size_t bits_per_word = sizeof(word_t) * 8;
  static constexpr size_t number_of_words = (Bits / bits_per_word) + (((Bits %
bits_per_word) == 0) ? 0 : 1);

 public:
  bool all_first(size_t n) const {
    {
      if (n > Bits) {
#ifdef RETURN_INSTEAD_TERMINATE
        return false;
#else
        std::terminate();
#endif
      }
      size_t i = 0;
      for (; n > bits_per_word; n -= bits_per_word, i++) {
        if (words_[i] != ~word_t{0}) {
          return false;
        }
      }
      word_t last_word = words_[i];
      for (; n != 0; n--) {
        if ((last_word & 1) != 1) {
          return false;
        }
        last_word >>= 1;
      }
      return true;
    }
  }

  void fill() noexcept {
      for (auto& word : words_) {
          word = ~word_t{0};
      }
  }

 private:
  std::array<word_t, number_of_words> words_{};
};

volatile int X = 0;

int main() {
  if (X == 1) {
    bitset<123> bitset;
    static_cast<void>(bitset.all_first(123));
  } else {
    bitset<256> bitset;
    bitset.fill();
    if (!bitset.all_first(255)) {
      return 1;
    }
  }
  return 0;
}

```

See: https://gcc.godbolt.org/z/bEexjrKP4

This issue does not exist in GCC 10 or GCC > 12.1. I couldn't test if it does
work in GCC 11.3.1 (or the trunk of it).

Additional:
* I could also trigger the issue with -O1 -fipa-icf  -fpartial-inlining 
* If we do a return false instead of a std::terminate, no wrong code is
generated.

I am sorry, but I couldn't reduced the code any further - this already took so
much time to figure out it is a compiler bug.

[Bug c++/106921] [11/12.1] -O1 and -fipa-icf -fpartial-inlining causes wrong code

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |11.3.0
           Keywords|                            |needs-bisection, wrong-code
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-09-13
      Known to work|                            |10.4.0
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.

[Bug c++/106921] [11/12/13 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection             |
                 CC|                            |marxin at gcc dot gnu.org
             Status|NEW                         |ASSIGNED
            Summary|[11/12.1] -O1 and -fipa-icf |[11/12/13 Regression] -O1
                   | -fpartial-inlining causes  |and -fipa-icf
                   |wrong code                  |-fpartial-inlining causes
                   |                            |wrong code since
                   |                            |r11-4987-g602c6cfc79ce4ae6
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r11-4987-g602c6cfc79ce4ae6. Lemme take a look.

[Bug c++/106921] [11/12/13 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[lutztonineubert at gmail dot com]

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

--- Comment #3 from Toni Neubert <lutztonineubert at gmail dot com> ---
Any update on this? Which compiler flag triggers this bug exactly? Or is it a
complex combination multiple?

[Bug c++/106921] [11/12/13 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
Will take a look in next weeks.

[Bug c++/106921] [11/12/13 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.4
           Priority|P3                          |P2

[Bug c++/106921] [11/12/13 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
I've created a bit reduced test-case:

cat pr106921.c
#include <array>
#include <cstddef>
#include <exception>

template <size_t Bits>
class bitset {
 private:
  using word_t = size_t;
  static constexpr size_t bits_per_word = sizeof(word_t) * 8;

 public:
  void foo(size_t n) const {
    {
      if (n > Bits)
        std::terminate();

      size_t i = 0;
      for (; n > bits_per_word; n -= bits_per_word, i++)
      {
        __builtin_printf ("words[0]=%x, expected=%x\n", words_[i], ~word_t{});
        if (words_[i] != ~word_t{0})
          __builtin_abort ();
      }
    }
  }

  void fill() noexcept
  {
      for (auto& word : words_) {
          word = ~word_t{0};
      }
  }

 private:
  std::array<word_t, 2> words_{};
};

volatile int X = 0;

int main()
{
    bitset<1> bitset1;
    bitset1.foo(1);

    bitset<66> bitset2;
    bitset2.fill();

    bitset2.foo(65);

  return 0;
}

So what happens? First, a split part is created and ICF merges the functions:

void bitset<1>::_ZNK6bitsetILm1EE3fooEm.part.0 (const struct bitset * const
this, size_t n)
{
  size_t i;
  const value_type & D.14201;
  const value_type & D.14200;
  long unsigned int _3;
  long unsigned int _4;

  <bb 7> [local count: 1073741824]:
  goto <bb 5>; [100.00%]

  <bb 2> [local count: 0]:
  _3 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  __builtin_printf ("words[0]=%x, expected=%x\n", _3, 18446744073709551615);
  _4 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  if (_4 != 18446744073709551615)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  __builtin_abort ();

  <bb 4> [local count: 0]:
  n_6 = n_5 + 18446744073709551552;
  i_7 = i_2 + 1;

  <bb 5> [local count: 1073741824]:
  # n_5 = PHI <n_6(4), n_8(D)(7)>
  # i_2 = PHI <i_7(4), 0(7)>
  if (n_5 > 64)
    goto <bb 2>; [0.00%]
  else
    goto <bb 6>; [100.00%]

  <bb 6> [local count: 1073741824]:
  return;

}


void bitset<66>::_ZNK6bitsetILm66EE3fooEm.part.0 (const struct bitset * const
this, size_t n)
{
  size_t i;
  const value_type & D.14216;
  const value_type & D.14215;
  long unsigned int _3;
  long unsigned int _4;

  <bb 7> [local count: 536870913]:
  goto <bb 5>; [100.00%]

  <bb 2> [local count: 536870913]:
  _3 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  __builtin_printf ("words[0]=%x, expected=%x\n", _3, 18446744073709551615);
  _4 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  if (_4 != 18446744073709551615)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  __builtin_abort ();

  <bb 4> [local count: 536870913]:
  n_6 = n_5 + 18446744073709551552;
  i_7 = i_2 + 1;

  <bb 5> [local count: 1073741824]:
  # n_5 = PHI <n_6(4), n_8(D)(7)>
  # i_2 = PHI <i_7(4), 0(7)>
  if (n_5 > 64)
    goto <bb 2>; [50.00%]
  else
    goto <bb 6>; [50.00%]

  <bb 6> [local count: 536870913]:
  return;

}

I don't see there any problem, later on, the functions are inlined back and we
end up with the following in a-pr106921.c.094t.fixup_cfg3:

;; Function bitset<1>::_ZNK6bitsetILm1EE3fooEm.part.0
(_ZNK6bitsetILm1EE3fooEm.part.0, funcdef_no=539, decl_uid=14195,
cgraph_uid=120, symbol_order=148) (executed once)

void bitset<1>::_ZNK6bitsetILm1EE3fooEm.part.0 (const struct bitset * const
this, size_t n)
{
  size_t i;
  const value_type & D.14201;
  const value_type & D.14200;
  long unsigned int _3;
  long unsigned int _4;

  <bb 7> [local count: 1073741824]:
  goto <bb 5>; [100.00%]

  <bb 2> [local count: 0]:
  _3 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  __builtin_printf ("words[0]=%x, expected=%x\n", _3, 18446744073709551615);
  _4 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  if (_4 != 18446744073709551615)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  __builtin_abort ();

  <bb 4> [local count: 0]:
  n_6 = n_5 + 18446744073709551552;
  i_7 = i_2 + 1;

  <bb 5> [local count: 1073741824]:
  # n_5 = PHI <n_6(4), n_8(D)(7)>
  # i_2 = PHI <i_7(4), 0(7)>
  if (n_5 > 64)
    goto <bb 2>; [0.00%]
  else
    goto <bb 6>; [100.00%]

  <bb 6> [local count: 1073741824]:
  return;

}



;; Function main (main, funcdef_no=525, decl_uid=13979, cgraph_uid=106,
symbol_order=122) (executed once)

int main ()
{
  value_type * __for_begin;
  struct bitset bitset2;
  struct bitset bitset1;

  <bb 2> [local count: 357878152]:
  bitset1 = {};
  bitset<1>::_ZNK6bitsetILm1EE3fooEm.part.0 (&bitset1, 1);
  bitset2 = {};
  goto <bb 4>; [100.00%]

  <bb 3> [local count: 715863673]:
  MEM[(long unsigned int &)__for_begin_6] = 18446744073709551615;
  __for_begin_7 = __for_begin_6 + 8;

  <bb 4> [local count: 1073741824]:
  # __for_begin_6 = PHI <&MEM[(struct array *)&bitset2]._M_elems(2),
__for_begin_7(3)>
  if (&MEM <struct bitset> [(void *)&bitset2 + 16B] != __for_begin_6)
    goto <bb 3>; [66.67%]
  else
    goto <bb 5>; [33.33%]

  <bb 5> [local count: 357878152]:
  bitset<66>::_ZNK6bitsetILm66EE3fooEm.part.0 (&bitset2, 65);
  bitset1 ={v} {CLOBBER(eol)};
  bitset2 ={v} {CLOBBER(eol)};
  return 0;

}

Which seems correct as the abort guard is based on:

  _4 = MEM <const struct array> [(const value_type &)this_1(D)]._M_elems[i_2];
  if (_4 != 18446744073709551615)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  __builtin_abort ();

which is equivalent to MEM <const struct array> [(const value_type
&)this_1(D)]._M_elems[0] != -1 for n > 64.
For some reason, probably due to corrupted aliasing info, we eventually
optimize out the 'if (_4 != 18446744073709551615)'
statement and we end up with abort.

@Honza, may I ask you for a help, please?

[Bug ipa/106921] [11/12/13 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[tkapela at poczta dot fm]

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

Tomasz Kapela <tkapela at poczta dot fm> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tkapela at poczta dot fm

--- Comment #6 from Tomasz Kapela <tkapela at poczta dot fm> ---
I think I encounter the same issue with my library code. Simplified example is
here 

https://github.com/tomasz-kapela/DMapProblem


Incorrect output appears only with -O2 flag on GCC 10, 11, 12.

Also turning off partial inlining optimalization solves the problem: 
-O2 -fno-partial-inlining 
I have checked that on GCC 8.4 with -O2 the output is correct.


Replacing lines 75-78 in include/capd/autodiff/EvalSub.h:

   if(coeffNo)
      result[coeffNo] = -right[coeffNo];
   else
      *result = *left - *right;

with

   result[coeffNo] = (coeffNo!=0) ? -right[coeffNo] : *left - *right;

produce correct output.

Should I post it as a separate bug report?

[Bug ipa/106921] [11/12/13/14 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|11.4                        |11.5

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 11.4 is being released, retargeting bugs to GCC 11.5.

[Bug ipa/106921] [11/12/13/14 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[lutztonineubert at gmail dot com]

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

--- Comment #8 from Toni Neubert <lutztonineubert at gmail dot com> ---
Hello,

I just wanted to ask what the state of this bug is? 

I think that incorrectly compiled code should be much more important than
anything else since any system can be affected without even knowing it.

I know that a lot of things happen here on a voluntary basis and I value that
very much.

What can we do to give such bugs a hire priority or help to solve such things?

[Bug ipa/106921] [11/12/13/14 Regression] -O1 and -fipa-icf -fpartial-inlining causes wrong code since r11-4987-g602c6cfc79ce4ae6

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|ASSIGNED                    |RESOLVED

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #5)
> So what happens? First, a split part is created and ICF merges the functions:
...
> 
> I don't see there any problem, later on, the functions are inlined back and
> we end up with the following in a-pr106921.c.094t.fixup_cfg3:

You missed that the range infomation on the SSA names are kept for one version
of the functions which meant they will be an inconsistency.

Anyways this is a dup of bug 113907 which has more analysis on the issue and
ideas of how to fix it.

*** This bug has been marked as a duplicate of bug 113907 ***