Page MenuHomePhabricator

Remove MachineLoopInfo dependency from AsmPrinter.

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



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

Diff Detail


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
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.
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?


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.


This revision was automatically updated to reflect the committed changes.