Page MenuHomePhabricator

Remove MachineLoopInfo dependency from AsmPrinter.
ClosedPublic

Authored by mzolotukhin on Mar 22 2018, 4:57 PM.

Details

Summary

Currently MachineLoopInfo is used in only two places:

  1. for computing IsBasicBlockInsideInnermostLoop field of MCCodePaddingContext, and it is never used.
  2. in emitBasicBlockLoopComments, which is called only if isVerbose() is true.

Despite that, we currently have a dependency on MachineLoopInfo, which makes
pass manager to compute it and MachineDominator Tree. This patch removes the
use (1) and makes the use (2) lazy, thus avoiding some redundant
recomputations.

Diff Detail

Repository
rL LLVM

Event Timeline

mzolotukhin created this revision.Mar 22 2018, 4:57 PM
espindola edited reviewers, added: espindola; removed: rafael.Mar 28 2018, 4:13 PM
espindola edited reviewers, added: chandlerc; removed: espindola.Mar 28 2018, 5:28 PM
  • Don't recompute MLI and MDT on visit of every basic block.
  • Rebase.
rengolin added inline comments.Apr 4 2018, 4:26 PM
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
2774 ↗(On Diff #141064)

Wait, isn't this only set if isVerbose?

mzolotukhin marked an inline comment as done.Apr 4 2018, 6:19 PM
mzolotukhin added inline comments.
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
2774 ↗(On Diff #141064)

Yes, and here it's also under this condition. Am I missing something?

rengolin accepted this revision.Apr 4 2018, 11:12 PM

I had seen the first version of this patch and thought it confusing, so refrained from reviewing (too broad context), but this version is much clearer now, and I can see it as straightforward benefit.

I'll tentatively approve waiting for another review for a day or so?

Thanks!

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
2774 ↗(On Diff #141064)

D'oh! Sorry, it is. Late night patch review.

This revision is now accepted and ready to land.Apr 4 2018, 11:12 PM
mzolotukhin marked an inline comment as done.Apr 5 2018, 10:55 AM

Thanks, Renato! I'll wait a little to give other reviewers more time to take a look.

Michael

This revision was automatically updated to reflect the committed changes.