[Bug tree-optimization/98464] New: [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825

[Bug tree-optimization/98464] New: [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825

[asolokha at gmx dot com]

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

            Bug ID: 98464
           Summary: [11 Regression] ICE: tree check: expected class
                    'type', have 'exceptional' (error_mark) in
                    tree_nop_conversion_p, at tree.c:12825
           Product: gcc
           Version: 11.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: ---

Created attachment 49847
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49847&action=edit
Testcase

gcc-11.0.0-alpha20201220 snapshot (g:18e86fae2a14f78e70aae06afce6bb9853068bb1)
ICEs when compiling the attached testcase, partially reduced from
test/std/containers/sequences/vector/vector.modifiers/insert_iter_rvalue.pass.cpp
from the libcxx 11.0.0 testsuite, w/ -O3 -fno-tree-dce:

% g++-11.0.0 -O3 -fno-tree-dce -c w93faca0.cpp
during GIMPLE pass: fre
w93faca0.cpp: In function 'int main()':
w93faca0.cpp:144:10: internal compiler error: tree check: expected class
'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at
tree.c:12825
  144 |      int main() {
      |          ^~~~
0x807515 tree_class_check_failed(tree_node const*, tree_code_class, char
const*, int, char const*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree.c:9860
0x80c768 tree_class_check(tree_node const*, tree_code_class, char const*, int,
char const*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree.h:3717
0x80c768 tree_nop_conversion_p(tree_node const*, tree_node const*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree.c:12825
0x16bfdf3 tree_nop_convert(tree_node*, tree_node**)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/build/gcc/generic-match.c:20
0x16dded3 generic_simplify_MINUS_EXPR
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/build/gcc/generic-match.c:29343
0xd7e350 fold_binary_loc(unsigned int, tree_code, tree_node*, tree_node*,
tree_node*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/fold-const.c:10756
0xd9f504 fold(tree_node*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/fold-const.c:13327
0x122116f simplify_replace_tree(tree_node*, tree_node*, tree_node*, tree_node*
(*)(tree_node*, void*), void*, bool)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-ssa-loop-niter.c:1970
0x122116f simplify_replace_tree(tree_node*, tree_node*, tree_node*, tree_node*
(*)(tree_node*, void*), void*, bool)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-ssa-loop-niter.c:1970
0x1222167 substitute_in_loop_info(loop*, tree_node*, tree_node*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-ssa-loop-niter.c:4997
0x10fb8b8 replace_uses_by(tree_node*, tree_node*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-cfg.c:2028
0x10fbc0f gimple_merge_blocks
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-cfg.c:2095
0xc26b1f merge_blocks(basic_block_def*, basic_block_def*)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/cfghooks.c:803
0x110c691 cleanup_tree_cfg_bb
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-cfgcleanup.c:774
0x110cd58 cleanup_tree_cfg_noloop
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-cfgcleanup.c:1103
0x110cd58 cleanup_tree_cfg(unsigned int)
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/tree-cfgcleanup.c:1187
0xfe0534 execute_function_todo
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/passes.c:2008
0xfe0e6b execute_todo
       
/var/tmp/portage/sys-devel/gcc-11.0.0_alpha20201227/work/gcc-11-20201227/gcc/passes.c:2096

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11 Regression] ICE: tree   |[11 Regression] ICE: tree
                   |check: expected class       |check: expected class
                   |'type', have 'exceptional'  |'type', have 'exceptional'
                   |(error_mark) in             |(error_mark) in
                   |tree_nop_conversion_p, at   |tree_nop_conversion_p, at
                   |tree.c:12825                |tree.c:12825 by r11-4637
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |linkw at gcc dot gnu.org
   Last reconfirmed|                            |2020-12-28

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> ---
It is caused by r11-4637.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[linkw at gcc dot gnu.org]

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

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |linkw at gcc dot gnu.org

--- Comment #2 from Kewen Lin <linkw at gcc dot gnu.org> ---
Thanks for reporting and bisecting, I'll investigate this first.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[linkw at gcc dot gnu.org]

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

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #3 from Kewen Lin <linkw at gcc dot gnu.org> ---
My commit exposed one issue in FRE. The reason why the ICE occurs is that one
SSA_NAME used in loop's nb_iterations is actually in free list.

(unsigned long) _54 - (unsigned long) v$ptr__25

    arg:0 <ssa_name 0x7ffff5549558 type <error_mark 0x7ffff54a1188>
        nothrow
        def_stmt
        version:54 in-free-list>>

I noticed that FRE already has some codes to handle loop's nb_iterations, but
there are some inconsistence which cause FRE unable to replace the one in
loop's nb_iterations but later replace all uses of _54.

When FRE is processing BB 19, it's able to replace _54 with _53 like:

  <bb 19> [local count: 789698041]:
  ...
  if (_26 != v$D4172$_M_impl$D4082$_M_finish$ptr__54)
    goto <bb 28>; [89.00%]

=>

  <bb 19> [local count: 789698041]:
  ...
  if (_26 != __new_finish$ptr__53)
    goto <bb 28>; [89.00%]

since it calls eliminate_stmt which works with def_bb instead of current bb:

    sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), use)

while rpo_vn_valueize only works with vn_context_bb.

For this particular case, _54's def_bb is BB 13, _53's def_bb is BB 11, it's ok
for dominated_by_p_w_unex check. While vn_context_bb is BB 19,
dominated_by_p_w_unex check returns false for it.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[linkw at gcc dot gnu.org]

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

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|linkw at gcc dot gnu.org           |rguenth at gcc dot gnu.org

--- Comment #4 from Kewen Lin <linkw at gcc dot gnu.org> ---
Posted one patch here[1], Richard pointed out it's incorrect and said he would
have a look later[2].

Hi Richi,

Passed this to you, thanks for your time in advance!

[1] https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562596.html
[2] https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562608.html

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0
           Priority|P3                          |P1

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
But this

        sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), use);

should make it more conservative (compared to the more desirable use of
gimple_bb (stmt) aka 'b').

The issue is really that dominated_by_p_w_unex is not "transitive" with
respect to an intermediate immediate dominator chain.

We can make that more consistent at least by doing sth like your patch but
only in vn_valueize_wrapper.  Longer term we need to think about a better
way to do what dominated_by_p_w_unex is doing, avoding the issue with the
intermediate dominators.  In particular somehow address

  /* Before iterating we'd like to know if there exists a
     (executable) path from bb2 to bb1 at all, if not we can
     directly return false.  For now simply iterate once.  */

it might be possible to compute that in an optimistic way for the non-iterating
scheme but for the iterative processing it needs some extra thought.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:39bd65faee3bafe2dc067e5fedb5079896551a8a

commit r11-6442-g39bd65faee3bafe2dc067e5fedb5079896551a8a
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Jan 4 13:02:24 2021 +0100

    tree-optimization/98464 - replace loop info with avail at uses

    This does VN replacement in loop nb_iterations consistent with
    the rest of the IL by using availability at the definition site
    of uses.

    2021-01-04  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/98464
            * tree-ssa-sccvn.c (vn_valueize_for_srt): Rename from ...
            (vn_valueize_wrapper): ... this.  Temporarily adjust vn_context_bb.
            (process_bb): Adjust.

            * g++.dg/opt/pr98464.C: New testcase.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[linkw at gcc dot gnu.org]

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

--- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #5)
> But this
> 
>         sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), use);
> 
> should make it more conservative (compared to the more desirable use of
> gimple_bb (stmt) aka 'b').
> 
> The issue is really that dominated_by_p_w_unex is not "transitive" with
> respect to an intermediate immediate dominator chain.
> 
> We can make that more consistent at least by doing sth like your patch but
> only in vn_valueize_wrapper.  

OK, just noticed that wrapper is only for simplify_replace_tree. Thanks for
fixing.  But I still don't get why it's buggy for rpo_vn_valueize.

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[rguenther at suse dot de]

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

--- Comment #9 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 4 Jan 2021, linkw at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98464
> 
> --- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #5)
> > But this
> > 
> >         sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), use);
> > 
> > should make it more conservative (compared to the more desirable use of
> > gimple_bb (stmt) aka 'b').
> > 
> > The issue is really that dominated_by_p_w_unex is not "transitive" with
> > respect to an intermediate immediate dominator chain.
> > 
> > We can make that more consistent at least by doing sth like your patch but
> > only in vn_valueize_wrapper.  
> 
> OK, just noticed that wrapper is only for simplify_replace_tree. Thanks for
> fixing.  But I still don't get why it's buggy for rpo_vn_valueize.

Because rpo_vn_valueize argument can be a value handle SSA which does
not need to be a definition on the path to the stmt we are simplifying.
rpo_vn_valueize is called from (too) many contexts and I'm not 100%
convinced we're only feeding it names available at the location we are
processing.

Note I think we should use 'b' and not SSA_NAME_DEF_STMT of the use but
then this code is as-is since the rewrite and I don't feel tracking down
fallout of changing this back now (but I'll try to remember for stage1,
as well as somehow better dealing with dominated_by_w_unex)

[Bug tree-optimization/98464] [11 Regression] ICE: tree check: expected class 'type', have 'exceptional' (error_mark) in tree_nop_conversion_p, at tree.c:12825 by r11-4637

[linkw at gcc dot gnu.org]

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

--- Comment #10 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #9)
> On Mon, 4 Jan 2021, linkw at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98464
> > 
> > --- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
> > (In reply to Richard Biener from comment #5)
> > > But this
> > > 
> > >         sprime = eliminate_avail (gimple_bb (SSA_NAME_DEF_STMT (use)), use);
> > > 
> > > should make it more conservative (compared to the more desirable use of
> > > gimple_bb (stmt) aka 'b').
> > > 
> > > The issue is really that dominated_by_p_w_unex is not "transitive" with
> > > respect to an intermediate immediate dominator chain.
> > > 
> > > We can make that more consistent at least by doing sth like your patch but
> > > only in vn_valueize_wrapper.  
> > 
> > OK, just noticed that wrapper is only for simplify_replace_tree. Thanks for
> > fixing.  But I still don't get why it's buggy for rpo_vn_valueize.
> 
> Because rpo_vn_valueize argument can be a value handle SSA which does
> not need to be a definition on the path to the stmt we are simplifying.
> rpo_vn_valueize is called from (too) many contexts and I'm not 100%
> convinced we're only feeding it names available at the location we are
> processing.
> 

Thanks for the explanation! I'll keep in mind of this kind of possibility.

> Note I think we should use 'b' and not SSA_NAME_DEF_STMT of the use but
> then this code is as-is since the rewrite and I don't feel tracking down
> fallout of changing this back now (but I'll try to remember for stage1,
> as well as somehow better dealing with dominated_by_w_unex)

Yeah, I totally agree that improving dominated_by_w_unex is better. The
existing checking in dominated_by_w_unex is limited, I did one hack on it by
recursing one more argument depth and special handling on backedge before I
went with the direction with SSA_NAME_DEF_STMT, but thought it can enlarge FRE
applied scope and also impact compilation time definitely, looks not a good
direction especially now is stage3.