Page MenuHomePhabricator

[ARM][LowOverheadLoops] Liveouts and reductions

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



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
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.


nit: Uses -> Defs?


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


nit: it's -> its

samparker added inline comments.Aug 26 2020, 11:43 PM

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

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

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



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.