> On Tue, Mar 13, 2012 at 7:31 PM, Andrew Pinski <pins...@gmail.com> wrote:
> > Ping?  Rebootstrapped on x86_64-linux-gnu with no regressions.
> Zdenek, can you have a look here?  I think the patch is reasonable, but
> you should have a better idea ;)

I do not understand the patch very well.  In the comment before the function,
please provide an explanation of what the heuristics mean (e.g., give examples
of what the particular cases are supposed to disambiguate).  Further

+/* Return the basic block where we might be doing an exit from a loop
+   if we go back up the cfg starting at basic block B skipping other loops
+   on the way and join points too.  */
+static basic_block
+skip_to_exit (basic_block b, struct loop *loop, unsigned succ_edge_count)

The comment describing this function is rather vague.  The succ_edge_count
parameter is not documented.  Also, skip_to_exit actually does not seem to do
anything with loop exits, so some less missleading name would be better.

+      /* There are multiple latches, we can't figure out the preheader,
+        just return b. */
+      if (oloop->latch == NULL)
+       return NULL;

The code does not seem to match the comment.

+      if (c == NULL)
+       return NULL;
+      d = skip_to_exit (EDGE_PRED (b, 1)->src, loop, 1);
+      if (c == NULL)
+       return NULL;
+      if (c != d)
+       return NULL;

The second "c == NULL" check is redundant.

+ ... accessor of the latch ...

What do you mean by "accessor"? 

+  if (VEC_length (edge, latches) != 1)
+    {

this condition is redundant, find_subloop_latch_edge_by_ivs will only be
called (and it only makes sense to call it) when there is more than
one latch edge candidate.

+       fprintf (dump_file, "no latches for IV subloob.\n");


+         fprintf (dump_file, "more one latches:");

"several latches:"

+      if (EDGE_COUNT (loop->header->succs) == 1)


Reply via email to