This is an archive of the discontinued LLVM Phabricator instance.

[LCSSA] Use IRBuilder for PHI creation.
ClosedPublic

Authored by fhahn on Jul 31 2020, 10:34 AM.

Details

Summary

Use IRBuilder instead PHINode::Create. This should not impact the
generated code, but IRBuilder provides a way to register callbacks for
inserted instructions, which is convenient for some users.

Diff Detail

Event Timeline

fhahn created this revision.Jul 31 2020, 10:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2020, 10:34 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
fhahn requested review of this revision.Jul 31 2020, 10:34 AM
lebedev.ri accepted this revision.Jul 31 2020, 10:43 AM

LGTM, but shouldn't the patch as-is should be marked as "NFC"?

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
1773

Why is this now needed? fixupLCSSAFormFor() already preserves insertion point.

This revision is now accepted and ready to land.Jul 31 2020, 10:43 AM
fhahn added a comment.Aug 1 2020, 10:04 AM

LGTM, but shouldn't the patch as-is should be marked as "NFC"?

Now all instructions inserted as part of LCSSA PHI node creation are marked as 'inserted', whereas before only the final ones were. There weren't any binary changes on SPEC2000/SPEC2006/MultiSource with -O3 -flto, but I guess there some users of the expander could also skip the inserted PHI nodes now. Extremely unlikely to cause any changes in the end, but it slightly changes what the expander considers as 'inserted'. Hence I'd prefer it not to be marked as NFC, but it's borderline I think.

llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
1773

Yeah, I'll drop it.

This revision was automatically updated to reflect the committed changes.