Page MenuHomePhabricator

[unroll] Split full exact and full bound unroll costing [NFC]

Authored by reames on Nov 23 2021, 9:41 AM.



This change should be NFC. It's posted for review mostly to make sure others are happy with the names I'm introducing for "exact full unroll" and "bounded full unroll". The motivation here is that our cost model for bounded unrolling is too aggressive - it gives benefits for exits we aren't going to prune - but I also just think the new version of the code is a lot easier to follow.

Diff Detail

Event Timeline

reames created this revision.Nov 23 2021, 9:41 AM
reames requested review of this revision.Nov 23 2021, 9:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2021, 9:41 AM
reames updated this revision to Diff 389240.Nov 23 2021, 9:47 AM

Further simplify - despite structure of existing code, shouldFullUnroll doesn't modify it's UP param.

I'm not sure i'm ready to comment on that whole logic, or whether the change is NFC, but the names don't seem particularly wrong there.

fhahn accepted this revision.Nov 25 2021, 3:21 AM

LGTM, thanks!


nit: it might also be a good opportunity to limit the scope of UnrollFactor to this block, e.g. by if (auto UnrollFactor = shouldFullUnroll(L, TTI, DT, SE, EphValues, TripCount, UCE, UP)) {}

This revision is now accepted and ready to land.Nov 25 2021, 3:21 AM
nikic added inline comments.Nov 25 2021, 9:35 AM

All copies of some exit test, not necessarily the latch exit test.

This revision was landed with ongoing or failed builds.Nov 29 2021, 2:19 PM
This revision was automatically updated to reflect the committed changes.