This is an archive of the discontinued LLVM Phabricator instance.

[ARM][MVE] Tail-predication: support constant trip count
ClosedPublic

Authored by SjoerdMeijer on Jan 22 2020, 7:37 AM.

Details

Summary

Let's also support the easy/easier case: constant trip counts :)

And while I was at it, a minor optimisation: I don't think we need to run Cleanup when there are no NewPredicates; it wasn't doing anything other than iterating over empty sets, but thought not calling it all was more obvious; I had to move the initialisation of ClonedVCTPInExitBlock for that.

Diff Detail

Event Timeline

SjoerdMeijer created this revision.Jan 22 2020, 7:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2020, 7:37 AM
samparker added inline comments.Jan 22 2020, 8:34 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
430–431

But the number of elements isn't equal to the trip count!

SjoerdMeijer added inline comments.Jan 22 2020, 10:31 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
430–431

Ha, the easy case, what a joke. Had myself convinced codegen was okay, but isn't. This indeed needs some more thinking how to approach this.

Thanks for catching that. This now gets the scalar loop iteration count from the icmp, which has as a RHS value a constant vector, i.e. a splat value with the scalar loop iteration count. This is returned as the computed number of elements for loops with constant tripcounts.

samparker added inline comments.Jan 23 2020, 7:02 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
379

Looks like this could just be in MatchICmpConstVec? Save us from calling MatchICmpConstVec twice and reduces the logic involved.

631

With those functions combined, we could then just pass the constant value to isTailPredicate. So something like:

Value *NumElements = nullptr
if (!MatchICmpConstVec(Predicate, NumElements) && !ComputeElements(Predicate))
  continue;

This would allow our logic to diverge early, but maybe only once, instead of weaving between the two in the other two functions.

Thanks for the suggestion, I have refactored things: we now have 2 functions ComputeRuntimeElements() and ComputeConstElements that match the number of elements and feed that to isTailPredicate(), so that none of this decision making has to be done in isTailPredicate() anymore.

Minor fix in the assert in ComputeConstElements which looked at the wrong constant.

samparker added inline comments.Jan 24 2020, 7:26 AM
llvm/lib/Target/ARM/MVETailPredication.cpp
243

Combine with ComputeRuntimeElements? I'm finding really hard to follow what's producing what and why some things are pointers and others of pointer pointers...

272

Doesn't look like you need ICmp or Shuffle here.

646

if MatchElemCountLoopSetup was merged with ComputeRuntimeElements, we can also get rid of the duplicated call to isTailPredicate and the lambda.

if (NumElements = ComputeConst...)
else if (NumElements = ComputeRuntime...)
else continue;

if (!isTailPredicate...) continue;
SjoerdMeijer updated this revision to Diff 240481.EditedJan 27 2020, 12:43 AM

Introduced a struct for the tripcount pattern matching bookkeeping, simplifying passing values around and the main loop driving the analysis.

samparker accepted this revision.Jan 27 2020, 2:57 AM

Nice. LGTM.

This revision is now accepted and ready to land.Jan 27 2020, 2:57 AM
This revision was automatically updated to reflect the committed changes.