Re: [PATCH] Cleanup back_threader::find_path_to_names.

[Jeff Law via Gcc-patches]

On 11/5/2021 9:09 AM, Aldy Hernandez wrote:
The main path discovery function was due for a cleanup.  First,
there's a nagging goto and second, my bitmap use was sloppy.  Hopefully
this makes the code easier for others to read.

Regstrapped on x86-64 Linux.  I also made sure there were no difference
in the number of threads with this patch.

No functional changes.

OK?

gcc/ChangeLog:

        * tree-ssa-threadbackward.c (back_threader::find_paths_to_names):
        Remove gotos and other cleanups.
---
  gcc/tree-ssa-threadbackward.c | 52 ++++++++++++++---------------------
  1 file changed, 20 insertions(+), 32 deletions(-)

diff --git a/gcc/tree-ssa-threadbackward.c b/gcc/tree-ssa-threadbackward.c
index b7eaff94567..d6a5b0b8da2 100644
--- a/gcc/tree-ssa-threadbackward.c
+++ b/gcc/tree-ssa-threadbackward.c
@@ -402,26 +402,18 @@ back_threader::find_paths_to_names (basic_block bb, 
bitmap interesting)
  
    m_path.safe_push (bb);
  
+  // Try to resolve the path without looking back.
    if (m_path.length () > 1
-      && !m_profit.profitable_path_p (m_path, m_name, NULL))
+      && (!m_profit.profitable_path_p (m_path, m_name, NULL)
+         || maybe_register_path ()))
      {
        m_path.pop ();
        m_visited_bbs.remove (bb);
        return false;
      }
  
-  // Try to resolve the path without looking back.
-  if (m_path.length () > 1 && maybe_register_path ())
-    {
-      m_path.pop ();
-      m_visited_bbs.remove (bb);
-      return true;
-    }
Hmm, note the return values are different in these cases, which you've 
merged to always return false.    I was about to suggest you just kill 
the return value and the cleanups that would enable.  But I see your 
patch uses the return value where we didn't before.

So I think that raises the question, for the recursive calls which set 
"done", does the change in return value, particularly when 
maybe_register_path returns true impact anything?

jeff