On 09/10/2015 05:14 PM, Segher Boessenkool wrote:
This patch rewrites the shrink-wrapping algorithm, allowing non-linear
pieces of CFG to be duplicated for use without prologue instead of just
linear pieces.

An example would be good for this kind of patch, also in the comments.


-      add_to_hard_reg_set (&hardregs, GET_MODE (dreg),
-                          REGNO (dreg));
+      add_to_hard_reg_set (&hardregs, GET_MODE (dreg), REGNO (dreg));
      }

In general please try to avoid mixing formatting changes with functional ones. No need to remove these ones from future versions of the patch though.

+  /* If there is more than one predecessor of PRO not dominated by PRO,
+     fail.  We don't have to do this (can split the block), but do this
+     for now (the original code disallowed this, too).

Comments shouldn't reference previous versions. Also, a comment describing the why rather than just what is being done would be more helpful.

I'm wondering how your new algorithm prevents the prologue from being placed inside a loop. Can you have a situation where this picks a predecessor that is reachable but not dominated by PRO?

+  int num = (*entry_edge)->probability;
+  int den = REG_BR_PROB_BASE;
+
+
+  if (*entry_edge == orig_entry_edge)
+    goto out;

There are a couple of places where there are multiple blank lines. I think we avoid that.

+       FOR_EACH_EDGE (e, ei, src->succs)
+         {
+           if (e->dest == EXIT_BLOCK_PTR_FOR_FN (cfun))
              {
-               all_set = false;
-               break;
              }

Minor, but I'd prefer a continue rather than an empty { } block.

Other than that it looks pretty good.


Bernd

Reply via email to