This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Bail out of mergeTruncStores when not optimizing
ClosedPublic

Authored by aeubanks on Oct 11 2021, 6:53 PM.

Details

Summary

With unoptimized code, we may see lots of stores and spend too much time in mergeTruncStores.

Fixes PR51827.

Diff Detail

Event Timeline

aeubanks created this revision.Oct 11 2021, 6:53 PM
aeubanks requested review of this revision.Oct 11 2021, 6:53 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2021, 6:53 PM

not really sure how to test this, pointers appreciated

I see similar checks sprinkled around DAGCombiner, so I guess this is ok too. Not sure if it is possible to do this at a higher level and remove those checks though.

But shouldn't this change the codegen for a reduced example of https://llvm.org/PR51827 ? Or is there some other optnone check that hides this one?

I see similar checks sprinkled around DAGCombiner, so I guess this is ok too. Not sure if it is possible to do this at a higher level and remove those checks though.

But shouldn't this change the codegen for a reduced example of https://llvm.org/PR51827 ? Or is there some other optnone check that hides this one?

This patch doesn't affect the codegen of the attached IR, but speeds it up a lot. It's probably bailing somewhere in mergeTruncStores() for most inputs.

spatel accepted this revision.Oct 20 2021, 5:03 AM

LGTM - it would be nicer if we had a higher-level bailout for optnone / -O0 instead of individual checks scattered around the combiner, but that might be (practically) impossible?

This revision is now accepted and ready to land.Oct 20 2021, 5:03 AM
This revision was landed with ongoing or failed builds.Oct 20 2021, 4:58 PM
This revision was automatically updated to reflect the committed changes.

A question I had while looking into this, why is this (and other store optimizations) being done in isel instead on the IR optimization side?