This is an archive of the discontinued LLVM Phabricator instance.

[LoopUnroll] Freeze tripcount rather than condition
ClosedPublic

Authored by nikic on May 18 2022, 8:42 AM.

Details

Summary

This is a followup to D125754. We introduce two branches, one before the unrolled loop and one before the epilogue (and similar for the prologue case). The previous patch only froze the condition on the first branch.

Rather than independently freezing the second condition, this patch instead freezes TripCount and bases BECount on it. These are the two quantities involved in the conditions, and this ensures that both work on a consistent, non-poisonous trip count.

Diff Detail

Event Timeline

nikic created this revision.May 18 2022, 8:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 8:42 AM
nikic requested review of this revision.May 18 2022, 8:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2022, 8:42 AM
reames added inline comments.May 18 2022, 8:47 AM
llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
752

Really not a fan of this. Your duplicating already subtle logic, and loosing the possibility of SCEV simplification on the BE count.

Doing it once makes sense, but maybe expand both and then freeze the results independently?

nikic added inline comments.May 18 2022, 9:33 AM
llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
752

I gave this a try, and seeing one alive failure on Transforms/LoopUnroll/runtime-loop-multiple-exits.ll with this variant: https://gist.github.com/nikic/82c3a9ee77141e813f7cd5362890c017

I'm having a hard try trying to parse the output, but I think what it is telling us is that because the values are independently frozen, it's possible to pick the values such that both the unrolled loop and the epilogue are skipped and @bar() never gets called.

So I think it's important that we freeze one value and base the other one on it, rather than freezing both independently.

nlopes added inline comments.May 19 2022, 6:24 AM
llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
752

Yeah, the counterexample produced by Alive2 isn't great. I'll look into that.

What it's saying is that in source we call @bar() and that function doesn't return.
In target, we don't do the function call and return instead. This is because %trip is frozen twice independently:

%0 = add i64 %trip, -1
%1 = freeze i64 %trip
%2 = freeze i64 %0

When %trip=undef, we may have %2 != %1 - 1.

nikic updated this revision to Diff 431325.May 23 2022, 4:24 AM

Update comment to clarify that TripCount/BECount cannot be frozen independently.

reames accepted this revision.May 23 2022, 9:41 AM

LGTM.

This revision is now accepted and ready to land.May 23 2022, 9:41 AM
This revision was landed with ongoing or failed builds.May 24 2022, 12:42 AM
This revision was automatically updated to reflect the committed changes.