This is an archive of the discontinued LLVM Phabricator instance.

[ExpandReductions] Don't push all intrinsics to the worklist. Just push reductions.
ClosedPublic

Authored by craig.topper on Oct 26 2019, 6:12 PM.

Details

Summary

We were previously pushing all intrinsics used in a function to the
worklist. This is wasteful for memory in a function with a lot of
intrinsics.

We also ask TTI if we should expand every intrinsic, but we only
have expansion support for the reduction intrinsics. This just
wastes time for the non-reduction intrinsics.

This patch only pushes reduction intrinsics into the worklist and
skips other intrinsics.

Event Timeline

craig.topper created this revision.Oct 26 2019, 6:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 26 2019, 6:12 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
RKSimon added inline comments.Oct 28 2019, 2:50 AM
llvm/lib/CodeGen/ExpandReductions.cpp
98

Do we gain anything by doing the if TTI->shouldExpandReduction(II) here to decide whether to add to WorkList?

simoll added a subscriber: simoll.Oct 28 2019, 7:33 AM
spatel added inline comments.Oct 28 2019, 7:47 AM
llvm/lib/CodeGen/ExpandReductions.cpp
81

Could reduce with something like:

for (auto &I : instructions(F)) {

Note - either way, this pass may be in danger of dying if used on unreachable blocks that contain weird IR.
See for example: D67766

craig.topper marked an inline comment as done.Nov 13 2019, 8:10 PM
craig.topper added inline comments.
llvm/lib/CodeGen/ExpandReductions.cpp
81

Does the UnreachableBlockEliminationPass that we run earlier in the codegen pipeline help prevent that?

Call shouldExpandReduction before putting in the worklist

This revision is now accepted and ready to land.Nov 13 2019, 10:43 PM
spatel accepted this revision.Nov 14 2019, 5:17 AM
spatel added inline comments.
llvm/lib/CodeGen/ExpandReductions.cpp
81

Nice - I didn't know that existed. It's just a wrapper around a util function: llvm::EliminateUnreachableBlocks().

So yes, that should make it pretty safe. Still a chance that fuzzers will target this pass in a different pipeline or that something between that running and this running would create a dead block with bogus code, but the odds are low.

This revision was automatically updated to reflect the committed changes.