This is an archive of the discontinued LLVM Phabricator instance.

[ARM][MVE] Validate tail predication values
ClosedPublic

Authored by samparker on Mar 2 2020, 6:51 AM.

Details

Summary

Iterate through the loop and check that the observable values produced are the same whether tail predication happens or not.
We want to find out if the tail-predicated version of this loop will produce the same values as the loop in its original form. For this to be true, the newly inserted implicit predication must not change the the results. All MVE loads and stores have to be predicated, so we know that any load operands, or stored results are equivalent already.
Other explicitly predicated instructions will perform the same operation in the original loop and the tail-predicated form. We call predicated instructions 'Known' and their users are also Known if it overwrites an Known operand. This is because tail predication will cause the false lanes to not be updated, but we know the output because we know the input.
For any 'Unknown' instructions, we can check that they're only consumed by Known instructions because it means that the unknown false lane(s) are replaced with known lane(s).
What this should result in is that all instructions are dependent upon predicated inputs. It also means that for an iteration of the loop, a register should hold the same value whether tail predication has happened or not, with the exception when the differences are masked away by their user(s) and not observable elsewhere.

Diff Detail

Event Timeline

samparker created this revision.Mar 2 2020, 6:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2020, 6:51 AM
samparker updated this revision to Diff 247886.Mar 3 2020, 6:55 AM
samparker retitled this revision from [ARM][MVE] Validate tail predication live-ins to [ARM][MVE] Validate tail predication values.
samparker edited the summary of this revision. (Show Details)

Rewrite.

samparker updated this revision to Diff 248180.Mar 4 2020, 8:22 AM
  • Renamed the sets.
  • Change comment describing what I'm trying to achieve.
SjoerdMeijer added inline comments.Mar 5 2020, 2:15 AM
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
555

Would it be good to spell out here that we have these 2 use cases?

  • IR produced by the vectoriser, and
  • IR originating from hand written (vector) intrinsics, which can also trigger tail-predication.

loads/stores produced by the vectoriser (when it thinks tail-folding is good) will all be nicely masked, so this concerns the 2nd use case. Is that right?

560

nit: analyse -> analysis?

574

I appreciate this is bikeshedding, but here we go anyway: UnknownFalseLanes and KnownFalseZeros is a bit difficult to read (in the code below), but your descriptions/comments above are clear. Summarising that, we have a set of instructions with some behaviour that we (immediately) understand, and there are some instructions that needs to be further analysed.
So, was wondering KnownFalseZeros could be named something along the lines of just Predicated, and UnknownFalseLanes be the Worklist of things to check?

589

can you use one of your new RDA helper functions for this?

samparker marked 3 inline comments as done.Mar 5 2020, 3:45 AM
samparker added inline comments.
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
555

This pass shouldn't be concerned with whether the input is from the vectorizer or the intrinsics, as we can't know. On line 762 we enforce that all MVE memory operations are explicitly predicated.

574

I did have Predicated at some point, but I thought that was misleading because some instructions won't be using the VPR. I like KnownFalseZeros because it's explicit in what we expect and I'm much rather have code that doesn't require a big comment to make any sense of what is going on. It doesn't have to be that, but its name should convey what's important about the set. The naming of UnknownFalseLanes is definitely less important though.

589

There were just static helpers and I don't think we won't really do enough of this logic to warrant more helpers.

was just wondering about more corner cases for "negative" tests: do we have test where lanes are swapped (if that makes sense)?

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
582

Is there a test for this?

llvm/test/CodeGen/Thumb2/LowOverheadLoops/unpredicated-min.mir
1 ↗(On Diff #248180)

nit: rename file unpredicated-min.mir -> unpredicated-max.mir?

samparker marked an inline comment as done.Mar 10 2020, 1:52 AM

do we have test where lanes are swapped (if that makes sense)?

I don't think there's one for this pass... but that was the reason for having a unit test for validForTailPredication, so we don't have to write a whole loop to test an instruction.

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
582

The only instructions that I can think of would be indexed load/stores, but they'll be caught by isVectorPredicated. If/When we can enable reductions, we could have an instruction producing two scalar values and then this logic will need to be reorganised a bit.

SjoerdMeijer accepted this revision.Mar 10 2020, 2:27 AM

LGTM, with one nit inline.

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
582

okidoki, then probably just remove this, or replace it with an assert?

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