[Bug tree-optimization/106923] New: [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512

[Bug tree-optimization/106923] New: [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512

[asolokha at gmx dot com]

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

            Bug ID: 106923
           Summary: [13 Regression] ICE in eliminate_unnecessary_stmts, at
                    tree-ssa-dce.cc:1512
           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 20220911 snapshot (g:0ea5e3f4542832b8da016b152695e64a2a386309) ICEs
when compiling the following testcase w/ -O1 -finline-small-functions
-fpartial-inlining --param max-inline-insns-single=1 --param
uninlined-function-insns=10000:

int n;

int
baz (void);

__attribute__ ((returns_twice)) int
bar (void)
{
  if (baz ())
    ++n;

  return 0;
}

int
foo (void)
{
  return bar ();
}

% gcc-13.0.0 -O1 -finline-small-functions -fpartial-inlining --param
max-inline-insns-single=1 --param uninlined-function-insns=10000 -c xe0blvxx.c
during GIMPLE pass: dce
xe0blvxx.c: In function 'bar':
xe0blvxx.c:7:1: internal compiler error: in eliminate_unnecessary_stmts, at
tree-ssa-dce.cc:1512
    7 | bar (void)
      | ^~~
0x771cd5 eliminate_unnecessary_stmts
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220911/work/gcc-13-20220911/gcc/tree-ssa-dce.cc:1512
0x771cd5 perform_tree_ssa_dce
       
/var/tmp/portage/sys-devel/gcc-13.0.0_p20220911/work/gcc-13-20220911/gcc/tree-ssa-dce.cc:1945

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[13 Regression] ICE in      |[13 Regression] ICE in
                   |eliminate_unnecessary_stmts |eliminate_unnecessary_stmts
                   |, at tree-ssa-dce.cc:1512   |, at tree-ssa-dce.cc:1512
                   |                            |since
                   |                            |r13-2518-ga262f969d6fd936f
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |13.0
   Last reconfirmed|                            |2022-09-13
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started since r13-2518-ga262f969d6fd936f.

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's a latent bug in function splitting.  When we outline a part of a
returns-twice function the outlined part retains the 'returns-twice' attribute
and thus now techincally the function calls setjmp.

DCE now correctly asserts that a setjmp cannot appear out of nowhere and if it
were there cfun->calls_setjmp should better indicate that.

The easiest fix appears to be to disallow splitting returns-twice functions
or amend check_forbidden_calls and forcefully clear the flag from the split
part.

Honza?

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Honza, can you please look at this?

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |hubicka at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54414
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54414&action=edit
gcc13-pr106923.patch

Untested fix.

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:5321d53279a60ee589a3c9779beb46503f9fc49f

commit r13-5728-g5321d53279a60ee589a3c9779beb46503f9fc49f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 7 10:34:45 2023 +0100

    ipa-split: Don't split returns_twice functions [PR106923]

    As discussed in the PR, returns_twice functions are rare/special beasts
    that need special treatment in the cfg, and inside of their bodies
    we don't know which part actually works the weird returns twice way
    (either in the fork/vfork sense, or in the setjmp) and aren't updating
    ab edges to reflect that.

    I think easiest is just to never split these, like we already never
    split noreturn or malloc functions.

    2023-02-07  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/106923
            * ipa-split.cc (execute_split_functions): Don't split returns_twice
            functions.

            * gcc.dg/pr106923.c: New test.

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/106923] [13 Regression] ICE in eliminate_unnecessary_stmts, at tree-ssa-dce.cc:1512 since r13-2518-ga262f969d6fd936f

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:b24acae8f4d315a5b071ffc2574ce91c7a0800ca

commit r14-2850-gb24acae8f4d315a5b071ffc2574ce91c7a0800ca
Author: Jan Hubicka <jh@suse.cz>
Date:   Fri Jul 28 09:48:34 2023 +0200

    loop-split improvements, part 2

    this patch fixes profile update in the first case of loop splitting.
    The pass still gives up on very basic testcases:

    __attribute__ ((noinline,noipa))
    void test1 (int n)
    {
      if (n <= 0 || n > 100000)
        return;
      for (int i = 0; i <= n; i++)
        {
          if (i < n)
            do_something ();
          if (a[i])
            do_something2();
        }
    }

    Here I needed to do the conditoinal that enforces sane value range of n.
    The reason is that it gives up on:
          !number_of_iterations_exit (loop1, exit1, &niter, false, true)
    and without the conditonal we get assumption that n>=0 and not INT_MAX.
    I think from overflow we shold derive that INT_MAX test is not needed and
since
    the loop does nothing for n<0 it is also just an paranoia.

    I am not sure how to fix this though :(.  In general the pass does not
really
    need to compute iteration count.  It only needs to know what direction the
IVs
    go so it can detect tests that fires in first part of iteration space.

    Rich, any idea what the correct test should be?

    In testcase:
      for (int i = 0; i < 200; i++)
        if (i < 150)
          do_something ();
        else
          do_something2 ();
    the old code did wrong update of the exit condition probabilities.
    We know that first loop iterates 150 times and the second loop 50 times
    and we get it by simply scaling loop body by the probability of inner test.

    With the patch we now get:

      <bb 2> [count: 1000]:

      <bb 3> [count: 150000]:    <- loop 1 correctly iterates 149 times
      # i_10 = PHI <i_7(8), 0(2)>
      do_something ();
      i_7 = i_10 + 1;
      if (i_7 <= 149)
        goto <bb 8>; [99.33%]
      else
        goto <bb 17>; [0.67%]

      <bb 8> [count: 149000]:
      goto <bb 3>; [100.00%]

      <bb 16> [count: 1000]:
      # i_15 = PHI <i_18(17)>

      <bb 9> [count: 49975]:    <- loop 2 should iterate 50 times but
                                   we are slightly wrong
      # i_3 = PHI <i_15(16), i_14(13)>
      do_something2 ();
      i_14 = i_3 + 1;
      if (i_14 != 200)
        goto <bb 13>; [98.00%]
      else
        goto <bb 7>; [2.00%]

      <bb 13> [count: 48975]:
      goto <bb 9>; [100.00%]

      <bb 17> [count: 1000]:   <- this test is always true becuase it is
                                  reached form bb 3
      # i_18 = PHI <i_7(3)>
      if (i_18 != 200)
        goto <bb 16>; [99.95%]
      else
        goto <bb 7>; [0.05%]

      <bb 7> [count: 1000]:
      return;

    The reason why we are slightly wrong is the condtion in bb17 that
    is always true but the pass does not konw it.

    Rich any idea how to do that?  I think connect_loops should work out
    the cas where the loop exit conditon is never satisfied at the time
    the splitted condition fails for first time.

    Before patch on hmmer we get a lot of mismatches:
    Profile report here claims:
    dump id |static mismat|dynamic mismatch                                    
|
            |in count     |in count                  |time                     
|
    lsplit  |      5    +5|   8151850567  +8151850567| 531506481006      
+57.9%|
    ldist   |      9    +4|  15345493501  +7193642934| 606848841056      
+14.2%|
    ifcvt   |     10    +1|  15487514871   +142021370| 689469797790      
+13.6%|
    vect    |     35   +25|  17558425961  +2070911090| 517375405715      
-25.0%|
    cunroll |     42    +7|  16898736178   -659689783| 452445796198       
-4.9%|
    loopdone|     33    -9|   2678017188 -14220718990| 330969127663            
|
    tracer  |     34    +1|   2678018710        +1522| 330613415364       
+0.0%|
    fre     |     33    -1|   2676980249     -1038461| 330465677073       
-0.0%|
    expand  |     28    -5|   2497468467  
-179511782|--------------------------|

    With patch

    lsplit  |      0      |            0             | 328723360744       
-2.3%|
    ldist   |      0      |            0             | 396193562452      
+20.6%|
    ifcvt   |      1    +1|     71010686    +71010686| 478743508522      
+20.8%|
    vect    |     14   +13|    697518955   +626508269| 299398068323      
-37.5%|
    cunroll |     13    -1|    489349408   -208169547| 257777839725      
-10.5%|
    loopdone|     11    -2|    402558559    -86790849| 201010712702            
|
    tracer  |     13    +2|    402977200      +418641| 200651036623       
+0.0%|
    fre     |     13      |    402622146      -355054| 200344398654       
-0.2%|
    expand  |     11    -2|    333608636   
-69013510|--------------------------|

    So no mismatches for lsplit and ldist and also lsplit thinks it improves
speed by
    2.3% rather than regressig it by 57%.

    Update is still not perfect since we do not work out that the second loop
    never iterates.

    Ifcft wrecks profile by desing since it insert conditonals with both arms
100%
    that will be eliminated later after vect.  It is not clear to me what
happens
    in vect though.

    Bootstrapped/regtested x86_64-linux, comitted.

    gcc/ChangeLog:

            PR middle-end/106923
            * tree-ssa-loop-split.cc (connect_loops): Change probability
            of the test preconditioning second loop to very_likely.
            (fix_loop_bb_probability): Handle correctly case where
            on of the arms of the conditional is empty.
            (split_loop): Fold the test guarding first condition to
            see if it is constant true; Set correct entry block
            probabilities of the split loops; determine correct loop
            eixt probabilities.

    gcc/testsuite/ChangeLog:

            PR middle-end/106293
            * gcc.dg/tree-prof/loop-split-1.c: New test.
            * gcc.dg/tree-prof/loop-split-2.c: New test.
            * gcc.dg/tree-prof/loop-split-3.c: New test.