[PATCH] D10717: Enhance loop rotation with existence of profile data in MachineBlockPlacement pass.

[Cong Hou]

congh added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:834-838
@@ +833,7 @@
+
+  // We calculate the benefit and cost of a loop rotation respectively, which
+  // are represented by BlockFrequency. When comparing the benefit/cost between
+  // two different rotations, we represent the actual benefit as benefit - cost,
+  // and check (benefit1 - cost1) > (benefit2 - cost2) from (benefit1 + cost2) >
+  // (benefit2 + cost1). Hopefully there is no overflow.
+  BlockFrequency BestRotationBenefit(0);
----------------
congh wrote:
> davidxl wrote:
> > congh wrote:
> > > congh wrote:
> > > > davidxl wrote:
> > > > > chandlerc wrote:
> > > > > > As above, I think it would be useful to have just cost, and to use the natural addition and subtraction to find the minimum cost rotation.
> > > > > > 
> > > > > > Also, "hopefully there is no overflow" seems a bad sign. We should *definitely* have no overflow, and I think it makes sense to add asserts and such to enforce that.
> > > > > Cong is actually computing the overall benefit and maximize it. 
> > > > > 
> > > > > Cong, It is equivalent to compute cost only -- simply change it to LoopExternalConnectionCost. For rotation with positive benefit in current patch, the connection cost will be 0, while for rotation with no benefit, the connection cost will be the branch freq to the head (or branch out freq)
> > > > The arithmetic operation defined in BlockFrequency already considers overflow, but it may return the saturated result, which may not be what we want. I may have to use the similar way that is used in getSumForBlock() to prevent potential overflow.
> > > David, I am not clear how LoopExternalConnectionCost is working: do we only need this cost, or do we have other cost/benefit? I ask this because there may exist more than one rotation that is beneficial.
> > For each loop rotation, the entry-connection cost will be the sum of  frequencies (multiplied by misfetch cost) of all incoming edges that are not 'fall through' edges. Similarly, the exit-connection cost can be computed.
> I just understand how to define LoopExternalConnectionCost: a loop rotation with the chain head that is not the loop header will have this cost, which actually is equal to the benefit I am now using. But I don't agree with your latest definition of it: we only need to consider the frequency of the potential fall-through edge from the header's predecessor to the header.
Also, I found it difficult to calculate the exit-connection cost: there may exist several exits in the loop, and at first we don't know how much cost should be given to each rotation. Even we find the largest exit frequency from another loop, we could not avoid subtract operation on costs (represented by BlockFrequency without this operation defined) when a rotation has a tail block with an exit edge that is less frequent.

I am now afraid that in order to unify cost and benefit we may pay more here.


http://reviews.llvm.org/D10717