This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Move the truncstore_merge combine to the LoadStoreOpt pass and add support for an extra case.
ClosedPublic

Authored by aemerson on Apr 12 2023, 9:56 AM.

Details

Summary

If we have set of mergeable stores of shifts, but the original source value being shifted
is wider than the merged size, we should still be able to merge if we truncate first. To do this
however we need to search for stores speculatively up the block, without knowing exactly how
many stores we should see before we stop. The old algorithm has to match an exact number of
stores to fit the wide type, or it dies. The new one will try to set the wide type to however
many stores we found in the upwards block traversal and use later checks to verify if they're
a valid mergeable set.

The reason I need to move this to LoadStoreOpt is because the combiner works going top down
inside a block, which means that we end up doing partial merges because we haven't seen all
the possible stores before we mutate the MIR. In LoadStoreOpt we can go bottom up.

As a side effect of this change, we also end up doing better on an existing test case (missing_store)
since we manage to do a partial merge there.

Diff Detail

Event Timeline

aemerson created this revision.Apr 12 2023, 9:56 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: hiraditya. · View Herald Transcript
aemerson requested review of this revision.Apr 12 2023, 9:56 AM

@arsenm Does AMDGPU need this combine? If so you'll need to add LoadStoreOpt to your pipeline somewhere.

aemerson accepted this revision.Apr 12 2023, 5:17 PM

Oops, I accidentally committed this in 29c851f4e2ff because I forgot to reset my branch before doing another commit.

If you think this is the wrong approach I’m happy to revert, otherwise I’ll leave it.

This revision is now accepted and ready to land.Apr 12 2023, 5:17 PM
aemerson closed this revision.Apr 12 2023, 5:17 PM