This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Preserve instruction name in replaceInstUsesWith()
ClosedPublic

Authored by nikic on Dec 16 2022, 12:49 AM.

Details

Summary

Currently InstCombine folds using the return replaceInstUsesWith(V, Builder.CreateFoo()) pattern do not preserve the original name of the instruction. To preserve the name, you either have to use something like return FooInst::Create(...) which is usually less nice, or go out of the way to preserve the name with takeName(). We often don't do that.

This patch instead preserves the name in replaceInstUsesWith() when replacing a named instruction with an unnamed instruction. To be conservative, I also added a zero-use check, which is a proxy for the case where the instruction was just created, rather than an existing one reused. Possibly we could drop that part.

As InstCombine tests are robust against renames this does not cause any test diffs, so I regenerated a random test to show the effects.

Diff Detail

Event Timeline

nikic created this revision.Dec 16 2022, 12:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 12:49 AM
nikic requested review of this revision.Dec 16 2022, 12:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 12:49 AM
spatel accepted this revision.Dec 16 2022, 4:45 AM

LGTM

Nice! The trade-off of using the Builder vs. raw create APIs always causes friction.
If this means we can use the Builder all the time, that would be great.

This revision is now accepted and ready to land.Dec 16 2022, 4:45 AM