[Bug tree-optimization/106896] New: [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339

[Bug tree-optimization/106896] New: [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339

[asolokha at gmx dot com]

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

            Bug ID: 106896
           Summary: [13 Regression] ICE in to_sreal_scale, at
                    profile-count.cc:339
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---

gcc 13.0.0 20220904 snapshot (g:20d30e737ad79dc36817e59f1676aa8bc0c6b325) ICEs
when compiling the following testcase w/ -O2:

int m, n;

__attribute__ ((cold)) void
bar (void);

void
foo (int *p)
{
  m = 1;
  while (m > 0)
    {
      for (n = 0; n < 2; ++n)
        {
        }

      bar ();

      while (m > 0)
        {
        }
    }

  *p = 0;
  *p = m;
}

% gcc-13.0.0 -O2 -c mqgbo1aa.c
during GIMPLE pass: slp
mqgbo1aa.c: In function 'foo':
mqgbo1aa.c:7:1: internal compiler error: in to_sreal_scale, at
profile-count.cc:339
    7 | foo (int *p)
      | ^~~
0x72c105 profile_count::to_sreal_scale(profile_count, bool*) const
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/profile-count.cc:339
0x11acae2 vect_slp_node_weight
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:329
0x11acae2 vect_optimize_slp_pass::start_choosing_layouts()
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:4684
0x11ad92b vect_optimize_slp_pass::run()
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:5545
0x11ada40 vect_optimize_slp(vec_info*)
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:5568
0x11ada40 vect_optimize_slp(vec_info*)
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:5564
0x11b5a9c vect_slp_analyze_bb_1
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:7289
0x11b5a9c vect_slp_region
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:7377
0x11b74db vect_slp_bbs
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:7568
0x11b7783 vect_slp_function(function*)
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vect-slp.cc:7656
0x11c06a1 execute
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220904/work/gcc-13-20220904/gcc/tree-vectorizer.cc:1532

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org
   Target Milestone|---                         |13.0

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #1 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
The problematic line is:

  return bb->count.to_sreal_scale (ENTRY_BLOCK_PTR_FOR_FN (cfun)->count);

where bb->count is:

  {m_val = 0, m_quality = GUESSED_LOCAL}

and the entry block count is:

  {m_val = 0, m_quality = PRECISE}

So we fail the first assert here:

  /* Watch for cases where one count is IPA and other is not.  */
  if (in.ipa ().initialized_p ())
    {
      gcc_checking_assert (ipa ().initialized_p ());
      /* If current count is inter-procedurally 0 and IN is inter-procedurally
         non-zero, return 0.  */
      if (in.ipa ().nonzero_p ()
          && !ipa().nonzero_p ())
        return 0;
    }
  else
    /* We can handle correctly 0 IPA count within locally estimated
       profile, but otherwise we are lost and this should not happen.   */
    gcc_checking_assert (!ipa ().initialized_p () || !ipa ().nonzero_p ());

But ipa() says:

  /* Return variant of profile count which is always safe to compare
     across functions.  */
  profile_count ipa () const
    {
      if (m_quality > GUESSED_GLOBAL0_ADJUSTED)
        return *this;
      if (m_quality == GUESSED_GLOBAL0)
        return zero ();
      if (m_quality == GUESSED_GLOBAL0_ADJUSTED)
        return adjusted_zero ();
      return uninitialized ();
    }

So it feels like the ipa() in to_sreal_scale is a speculative
“could I compare this value across functions, if I had to?”.

The bb->count computation quoted above is deliberately intraprocedural
rather than interprocedural.  In that context it doesn't really seem
relevant that one of the values could be compared across functions,
since that isn't what we're doing.

What's the right fix here?

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2022-09-12
            Summary|[13 Regression] ICE in      |[13 Regression] ICE in
                   |to_sreal_scale, at          |to_sreal_scale, at
                   |profile-count.cc:339        |profile-count.cc:339 since
                   |                            |r13-2288-g61c4c989034548f4
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
Btw. started with r13-2288-g61c4c989034548f4.

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[marxin at gcc dot gnu.org]

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

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
I think you may want to use something like:
bb->count.probability_in (ENTRY_BLOCK_PTR_FOR_FN (cfun)->count).to_sreal()

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[rsandifo at gcc dot gnu.org]

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

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #3)
> I think you may want to use something like:
> bb->count.probability_in (ENTRY_BLOCK_PTR_FOR_FN (cfun)->count).to_sreal()
I'm not sure modelling it as a probability would work.  In general,
a bb can be executed more often than the entry block (e.g. for loops)
or less often (e.g. if the whole function is a diamond).  Modelling it
as a probability would cap one of the two cases to 1 (i.e. same as
equal counts).

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[marxin at gcc dot gnu.org]

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

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
> Modelling it
> as a probability would cap one of the two cases to 1 (i.e. same as
> equal counts).

You are correct, Honza should answer here!

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Honza?

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |shaohua.li at inf dot ethz.ch

--- Comment #7 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
*** Bug 107387 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2022-09-12 00:00:00         |2022-12-19

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Just a quick note, for some reason using the original testcase with the C++
front-end does not produce the ICE while using the C front-end does. I didn't
look into the IR difference or predict differences of why though.

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Honza, ping!

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[hubicka at gcc dot gnu.org]

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

--- Comment #10 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
The problem the assert is trying to solve is that local counters are all
frequencies relative to the entry block count, while IPA counters are absolute
values within the whole program. So comparing them mixes apples and oranges.

Normally if function has IPA profile, all counts within the function body
should be IPA.  Problem here is that this that we propagated the fact that
function always callls cold function into the entry block but somehow we mark
the first basic block as estimated locally which does not make sense.

This is inconsistency coming from updates after pcom.
We need to live with inconsistent profiles (since it is not always possible to
make them fully right) so I will patch to_sreal to simply give up and return 1
here.  That is what we do for unknown probabilities.

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[hubicka at gcc dot gnu.org]

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

--- Comment #11 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Originally to_sreal_frquency was intended to work both inter-procedurally and
intra-procedurally. However in such setup there are side cases that can not be
solved without knowing the corresponding entry_bb_counts which are not known to
the function.
Inspecting existing uses of to_sreal_frequency it is used intra-procedurally
only.  In that case it makes sense to implement it same way as probability_in
which is done by the following patch:

diff --git a/gcc/profile-count.cc b/gcc/profile-count.cc
index d05fef3fb00..16585045f8e 100644
--- a/gcc/profile-count.cc
+++ b/gcc/profile-count.cc
@@ -325,6 +325,13 @@ profile_count::to_cgraph_frequency (profile_count
entry_bb_count) const
 sreal
 profile_count::to_sreal_scale (profile_count in, bool *known) const
 {
+  if (*this == zero ()
+      && !(in == zero ()))
+  {
+    if (known)
+      *known = true;
+    return 0;
+  }
   if (!initialized_p () || !in.initialized_p ())
     {
       if (known)
@@ -333,32 +340,13 @@ profile_count::to_sreal_scale (profile_count in, bool
*known) const
     }
   if (known)
     *known = true;
-  /* Watch for cases where one count is IPA and other is not.  */
-  if (in.ipa ().initialized_p ())
-    {
-      gcc_checking_assert (ipa ().initialized_p ());
-      /* If current count is inter-procedurally 0 and IN is inter-procedurally
-        non-zero, return 0.  */
-      if (in.ipa ().nonzero_p ()
-         && !ipa().nonzero_p ())
-       return 0;
-    }
-  else 
-    /* We can handle correctly 0 IPA count within locally estimated
-       profile, but otherwise we are lost and this should not happen.   */
-    gcc_checking_assert (!ipa ().initialized_p () || !ipa ().nonzero_p ());
-  if (*this == zero ())
-    return 0;
-  if (m_val == in.m_val)
+  if (*this == in)
     return 1;
   gcc_checking_assert (compatible_p (in));
-
+  if (m_val == in.m_val)
+    return 1;
   if (!in.m_val)
-    {
-      if (!m_val)
-       return 1;
-      return m_val * 4;
-    }
+    return m_val * 4;
   return (sreal)m_val / (sreal)in.m_val;
 }

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jan Hubicka <hubicka@gcc.gnu.org>:

https://gcc.gnu.org/g:5159a1f1e91e03d4b82808a0062697318232543f

commit r13-6652-g5159a1f1e91e03d4b82808a0062697318232543f
Author: Jan Hubicka <jh@suse.cz>
Date:   Tue Mar 14 09:10:35 2023 +0100

    Fix overactive sanity check in profile_count::to_sreal_scale

    As discussed in the PR log, profile_count::to_cgraph_frequency was
originally
    intended to work across function boundary and has some extra logic and
sanity
    check for that.  It is used only within single function and with current
    API it can not really work well globally, so this patch synchronizes its
    implementation with probability_in which does similar job but to determine
    relative probability.

    gcc/ChangeLog:

    2023-03-14  Jan Hubicka  <hubicka@ucw.cz>

            PR tree-optimization/106896
            * profile-count.cc (profile_count::to_sreal_scale): Synchronize
            implementatoin with probability_in; avoid some asserts.

[Bug tree-optimization/106896] [13 Regression] ICE in to_sreal_scale, at profile-count.cc:339 since r13-2288-g61c4c989034548f4

[hubicka at gcc dot gnu.org]

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

Jan Hubicka <hubicka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
Fixed.