This is an archive of the discontinued LLVM Phabricator instance.

[BPI] Apply invoke heuristic before loop branch heuristic
ClosedPublic

Authored by apilipenko on May 25 2018, 6:21 AM.

Details

Summary

Currently the loop branch heuristic is applied before the invoke heuristic which makes us overestimate the probability of the unwind destination of invokes inside loops. This in turn makes us grossly underestimate the frequencies of loops with invokes.

loop:
  %i = phi i32 [ 0, %entry ], [ %i.next, %invoke.cont ]
  invoke void @foo() to label %invoke.cont unwind label %lpad

invoke.cont:
  %i.next = add i32 %i, 1
  %cont = icmp ult i32 %i.next, %n
  br i1 %cont, label %loop, label %exit, !prof !{!"branch_weights", i32 9999, i32 1}
-analyze -branch-prob: 
 ...
 edge loop -> invoke.cont probability is 0x7c000000 / 0x80000000 = 96.88% [HOT edge]
 edge loop -> lpad probability is 0x04000000 / 0x80000000 = 3.12%
 ...

-analyze -block-freq:
 ...
 - loop: float = 31.901
 ...

Applying the invoke heuristic before the loop one makes the results more resonable:

-analyze -branch-prob: 
 ...
 edge loop -> invoke.cont probability is 0x7ffff800 / 0x80000000 = 100.00% [HOT edge]
 edge loop -> lpad probability is 0x00000800 / 0x80000000 = 0.00%	
 ...

-analyze -block-freq:
 ...
 - loop: float = 9905.6
 ...

Diff Detail

Repository
rL LLVM

Event Timeline

apilipenko created this revision.May 25 2018, 6:21 AM
vsk added a comment.May 25 2018, 10:16 AM

Thanks for the patch. Full disclaimer: I don't have much knowledge about this part of the codebase.

I've noticed that calcUnreachableHeuristics and calcColdCallHeuristics both avoid work when the terminator is an invoke. Do you think it would make more sense to handle invokes first, and to then assert that invokes aren't seen in the other heuristics? IMO it would reduce the amount of special-casing needed.

apilipenko updated this revision to Diff 149776.Jun 4 2018, 8:49 AM

The suggestion to apply calcInvokeHeuristics before calcUnreachableHeuristics and calcColdCallHeuristics makes sense.

I think this looks good, but I'm not familiar enough with BPI to give an official lgtm.

skatkov accepted this revision.Jun 4 2018, 7:29 PM

LGTM. If no one objects in 1-2 days, feel free to land.

This revision is now accepted and ready to land.Jun 4 2018, 7:29 PM
This revision was automatically updated to reflect the committed changes.