This is an archive of the discontinued LLVM Phabricator instance.

[ARM][LowOverheadLoops] Liveouts and reductions
ClosedPublic

Authored by samparker on Aug 26 2020, 4:58 AM.

Details

Summary

I've removed the code that tried to look for reduction patterns, since the vectorizer and isel can now produce predicated reductions within the loop body. This has required some reorganisation and fixes around live-out and predication checks, as well as looking for cases where an input/output is initialised to zero.
I've only changed one test, one_loop_add_add_v16i8 in reductions.ll, to represent what IR should now be generated, but the rest of those functions remain untouched.
A lot of other tests look much worse because tail predication is no longer happening (probably more correct though!) and we don't expect to see those types of reductions anymore. I didn't want to make a whole bunch of test changes because enough of the existing tests have already changed and I don't want those to get lost with lots of input change.

Diff Detail

Event Timeline

samparker created this revision.Aug 26 2020, 4:58 AM
samparker requested review of this revision.Aug 26 2020, 4:58 AM
samparker updated this revision to Diff 287966.Aug 26 2020, 7:06 AM

Think I've fixed the confusing cases of dodgy predication happening, when the vctp needs to be live in an exit block.

samparker edited the summary of this revision. (Show Details)Aug 26 2020, 7:07 AM
samparker added inline comments.Aug 26 2020, 7:09 AM
llvm/test/CodeGen/Thumb2/LowOverheadLoops/unpredload.ll
49 ↗(On Diff #287966)

@dmgreen I still need to look at this one.

samparker updated this revision to Diff 287970.Aug 26 2020, 7:45 AM

Fixed the 'bad' test case.

That's good amount of red/deletions!
Read this for the first time, and looks very reasonable. I have one question inline for now while I go over this again.

llvm/include/llvm/CodeGen/ReachingDefAnalysis.h
208

nit: Uses -> Defs?

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

Is this the only instruction that could zero initialise that def?

656

nit: it's -> its

samparker added inline comments.Aug 26 2020, 11:43 PM
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
643

It's the only way that I saw, but there's probably more. It's the common and easy case to catch though.

SjoerdMeijer added inline comments.Aug 27 2020, 6:50 AM
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
643

ah, because how we use IsZeroInit, we are on the safe side here, right? I missed that earlier.
Not sure, perhaps XOR-ing the same register is the other common case?

samparker added inline comments.Aug 27 2020, 11:50 PM
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
643

I don't think that's something I've seen, I guess that a move immediate is the obvious pattern to match in isel.

SjoerdMeijer accepted this revision.Aug 28 2020, 12:24 AM

LGTM

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

nit: it's

This revision is now accepted and ready to land.Aug 28 2020, 12:24 AM
This revision was automatically updated to reflect the committed changes.