On 10/1/22 12:55, Bernhard Reutner-Fischer wrote:
On Fri, 30 Sep 2022 17:32:34 -0600
Jeff Law <j...@ventanamicro.com> wrote:

+  /* This looks good from a CFG standpoint.  Now look at the guts
+     of PRED.  Basically we want to verify there are no PHI nodes
+     and no real statements.  */
+  if (! gimple_seq_empty_p (phi_nodes (pred)))
+    return false;
So, given the below, neither DEBUG nor labels do count towards an
empty seq [coming in from any PHI that is, otherwise it's a different
thing], which is a bit surprising but well, ok. It looks at PHI IL, so
probably yes. Allegedly that's what it is. Neat if that's true.

Right.  A forwarder block is allowed to have local labels, but not labels for nonlocal gotos, so we can and should ignore local labels.  Debug statements must be ignored or you can end up with differing code generation based on whether or not -g is enabled or not.

In some contexts PHIs are allowed in forwarders, in other contexts they are not.  In this specific case I doubt it matters because of the restrictions we put on the CFG, the predecessor block is restricted to a single incoming edge.  The only way that'll have a PHI is if the PHI became a degenerate during DOM.





+
+  gimple_stmt_iterator gsi;
+  for (gsi = gsi_last_bb (pred); !gsi_end_p (gsi); gsi_prev (&gsi))
+    {
+      gimple *stmt = gsi_stmt (gsi);
+
+      switch (gimple_code (stmt))
+       {
+         case GIMPLE_LABEL:
+           if (DECL_NONLOCAL (gimple_label_label (as_a <glabel *> (stmt))))
+             return false;
+           break;
+
+         case GIMPLE_DEBUG:
+           break;
+
+         default:
+           return false;
don't like, sounds odd. Are we sure there's no other garbage that can
manifest here? int meow=42;, and meow unused won't survive?, pragmas
neither or stuff ?

That would generate a real statement and would thus be rejected. If there's anything other than a local label or debug statements in the block, then it's rejected.



@@ -583,6 +656,62 @@ record_edge_info (basic_block bb)
                if (can_infer_simple_equiv && TREE_CODE (inverted) == EQ_EXPR)
                edge_info->record_simple_equiv (op0, op1);
              }
+
+         /* If this block is a single block loop, then we may be able to
+            record some equivalences on the loop's exit edge.  */
+         if (single_block_loop_p (bb))
+           {
+             /* We know it's a single block loop.  Now look at the loop
+                exit condition.  What we're looking for is whether or not
+                the exit condition is loop invariant which we can detect
+                by checking if all the SSA_NAMEs referenced are defined
+                outside the loop.  */
+             if ((TREE_CODE (op0) != SSA_NAME
+                  || gimple_bb (SSA_NAME_DEF_STMT (op0)) != bb)
+                 && (TREE_CODE (op1) != SSA_NAME
+                     || gimple_bb (SSA_NAME_DEF_STMT (op1)) != bb))
+               {
+                 /* At this point we know the exit condition is loop
+                    invariant.  The only way to get out of the loop is
+                    if never traverses the backedge to begin with.  This
s/if /if it /

Will fix.  THanks.



+                    implies that any PHI nodes create equivalances we can
that any threw me off asking for "that if any". Would have been nicer,
i think?

All PHIs at the target of the loop backedge in this case create an equivalence.  That's the whole point of the patch, to prove a set of circumstances that ultimately require all the PHIs on the loop backedge to create an equivalence on the loop exit.




+                    attach to the loop exit edge.  */
attach it to

Updated, slightly differently, but should be clearer.



+                 int alternative
bool

Sure.




+                   = (EDGE_PRED (bb, 0)->flags & EDGE_DFS_BACK) ? 1 : 0;
+
+                 gphi_iterator gsi;
+                 for (gsi = gsi_start_phis (bb);
+                      !gsi_end_p (gsi);
+                      gsi_next (&gsi))
+                   {
+                     /* If the other alternative is the same as the result,
+                        then this is a degenerate and can be ignored.  */
+                     if (dst == PHI_ARG_DEF (phi, !alternative))
+                       continue;
+
+                     /* Now get the EDGE_INFO class so we can append
+                        it to our list.  We want the successor edge
+                        where the destination is not the source of
+                        an incoming edge.  */
+                     gphi *phi = gsi.phi ();
+                     tree src = PHI_ARG_DEF (phi, alternative);
+                     tree dst = PHI_RESULT (phi);
+
+                     if (EDGE_SUCC (bb, 0)->dest
+                         != EDGE_PRED (bb, !alternative)->src)
by now, alternative would be easier to grok if it would have been spelled
from_backedge_p or something. IMHO.

Agreed it's a bit ugly.  Let me think about  how to clean this up.


Anyway, I've queued the updates.  I'll push them the next time I do a bootstrap & regression test.


jeff


Reply via email to