Page MenuHomePhabricator

Fix two bugs in loop unswitching related to trivial unswitching and the threshold parameter

Authored by broune on Jun 10 2015, 5:08 PM.



This change fixes two bugs in loop unswitching. This change causes an 81% speed-up on a benchmark that is based on EigenConvolutionKernel2D from Eigen3, where the lack of loop unswitching blocks hoisting of loads out of a nested loop.

First fix: Unswitching on trivial conditions should always happen regardless of the computed unswitching cost, as really the cost is zero. While there is code to make that happen, the logic that checks the unswitching cost against a threshold was moved to an earlier point (revision 147935) than the point where trivial unswitching is detected, so trivial unswitching is currently blocked by the cost threshold. This change fixes that.

Second fix: Before revision 147935 (from 2012-01-11), the threshold parameter was a per-loop threshold. So an unswitching happened only if the cost of the unswitching was less than the threshold. In an indirect way (and I believe unintentionally), the logic for this since then has been that the threshold is an over-all budget across all loops for all loop unswitching done by a given LoopUnswitch loop pass object. So if an unswitching with cost 100 happens in one function, that in effect reduces the threshold from 100 to 0 for the loops even in another function. This persists for the lifetime of that loop pass object. This makes no difference for most small examples but it is important for large examples. This revision fixes that.

I've tried to make the change minimally invasive while staying with what I think was the original intent of the code.

Diff Detail

Event Timeline

broune abandoned this revision.Jun 10 2015, 5:08 PM
broune updated this revision to Diff 27477.
broune retitled this revision from to Fix two bugs in loop unswitching related to trivial unswitching and the threshold parameter.
broune updated this object.
broune edited the test plan for this revision. (Show Details)
broune added reviewers: hfinkel, eli.friedman, meheff.
broune added a subscriber: dyatkovskiy.

Phabricator tells me to make a new revision instead, this time adding llvm-commits as a subscriber, so I'll do that.

The real, fixed revision is at D10376. Ignore this one.