This is an archive of the discontinued LLVM Phabricator instance.

[X86] combineX86ShufflesConstants - constant fold from target shuffles unless optsize = true
ClosedPublic

Authored by RKSimon on Nov 12 2021, 3:08 AM.

Details

Summary

Currently we only constant fold target shuffles if any of the sources has one use, or it would remove a variable shuffle mask - the aim being to avoid constant pool bloat.

This patch proposes we should constant fold by default and only limit this if optsize is enabled - I've added a basic test for this in vector-mul.ll (the pmuludq case is by far the most common), I can add other specific test cases if people need them.

This should permit further constant folding, break some instruction dependencies and help reduce shuffle port pressure.

Diff Detail

Event Timeline

RKSimon created this revision.Nov 12 2021, 3:08 AM
RKSimon requested review of this revision.Nov 12 2021, 3:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 3:08 AM

I think this makes sense, the more computations we not-repeatedly-do at run-time, the better.
Is there any understanding of the bloat it was guarding against?
Any clues as to how much does this grow the constant pool?

I think this makes sense, the more computations we not-repeatedly-do at run-time, the better.
Is there any understanding of the bloat it was guarding against?
Any clues as to how much does this grow the constant pool?

shufflevector already constant folds (all the time - it has no optsize limit which is a shame), so I expect most of the constant pool size to come from there already - the reason we see so much pmuludq code changes is because we create the target shuffles directly during lowering

lebedev.ri accepted this revision.Nov 12 2021, 4:35 AM

Looks reasonable to me, but may be best to wait for one more review.

This revision is now accepted and ready to land.Nov 12 2021, 4:35 AM
pengfei added inline comments.Nov 12 2021, 5:23 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
37762–37764

Fix the comments?

RKSimon updated this revision to Diff 386825.Nov 12 2021, 5:39 AM

Update comment

pengfei accepted this revision.Nov 12 2021, 5:48 AM

LGTM.

Thanks everyone

This revision was landed with ongoing or failed builds.Nov 12 2021, 6:05 AM
This revision was automatically updated to reflect the committed changes.