[Bug tree-optimization/98815] New: Redundant free_dominance_info in cgraph_node::analyze()

[Bug tree-optimization/98815] New: Redundant free_dominance_info in cgraph_node::analyze()

[fxue at os dot amperecomputing.com]

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

            Bug ID: 98815
           Summary: Redundant free_dominance_info in
                    cgraph_node::analyze()
           Product: gcc
           Version: tree-ssa
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fxue at os dot amperecomputing.com
  Target Milestone: ---

Since "execute_pass_list ()" has an action of clearing dominance information,
two free_dominance_info() after the line "execute_pass_list (cfun,
g->get_passes ()->all_lowering_passes)" seems to be redundant, and could be
removed.

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Thanks for the report.

It's likely more complicated as execute_pass_list calls free_dominance_info
conditionally.

  if (cfun && fn->cfg)
    {
      free_dominance_info (CDI_DOMINATORS);
      free_dominance_info (CDI_POST_DOMINATORS);
    }

Moreover, there are more callers of execute_pass_list than the only
cgraph_node::analyze. That said, I tend to close it as invalid.

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[fxue at os dot amperecomputing.com]

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

--- Comment #2 from Feng Xue <fxue at os dot amperecomputing.com> ---
If we step into free_dominance_info(dir), it is a wrapper of
free_dominance_info (cfun, dir), which means it assumes a non-NULL "cfun".
Additionally, please go through calling stack of free_dominance_info():

   free_dominance_info ()
     -> if (!dom_info_available_p ())
       -> dom_info_state (fn, dir) != DOM_NONE
          ->  if (!fn->cfg)
                return DOM_NONE;

This shows when "fn->cfg" is NULL, free_dominance_info() does nothing.
Above all, if conditional check "cfun && fn->cfg" is true, two calls to
free_dominance_info() in cgraph_node::analyze() are redundant, and if the check
is false, these calls are trival. So we could remove those calls in
cgraph_node::analyze(), not execute_pass_list().

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|INVALID                     |---
             Status|RESOLVED                    |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org
   Last reconfirmed|                            |2021-01-26
     Ever confirmed|0                           |1

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
All right, let me take a look.

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
I can confirm the patch survives bootstrap and regression tests.
I'm going to send it at the beginning of the next stage1.

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:021607e12cb9c40d0859b78490f44bb3f7da5812

commit r12-23-g021607e12cb9c40d0859b78490f44bb3f7da5812
Author: Martin Liska <mliska@suse.cz>
Date:   Tue Jan 26 12:55:52 2021 +0100

    Test simlified call in cgraph_node::analyze().

    gcc/ChangeLog:

            PR ipa/98815
            * cgraphunit.c (cgraph_node::analyze): Remove duplicate
            free_dominance_info calls.

[Bug ipa/98815] Redundant free_dominance_info in cgraph_node::analyze()

[marxin at gcc dot gnu.org]

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

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

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

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
Fixed.