This is an archive of the discontinued LLVM Phabricator instance.

[PM] Edit comments in PassManager.h.
ClosedPublic

Authored by jlebar on Dec 2 2016, 3:57 PM.

Details

Summary

This covers most of PassManager.h, up to the introduction of inner/outer
analysis proxies.

If there's a theme to these changes, it's simplifying the language. For
example:

  • PreservedAnalyses is a "set of analyses", not an "abstract set". "Abstract" doesn't have any particular meaning here.
  • "Build types for the concept types" becomes "define the concept types".
  • Instead of "data structures optimized for pointer-like types using the alignment-provided low bits", say "data structures that use the low bits of pointers."
  • "Clear the map pointing into the results list" becomes "Delete the map entries that point into the results list."

This patch also fixes a few places where we referred to "function" and
"module" pass/analysis managers, instead of the more abstract "IRUnitT"
PM/AMs we have now.

Event Timeline

jlebar updated this revision to Diff 80142.Dec 2 2016, 3:57 PM
jlebar retitled this revision from to [PM] Edit comments in PassManager.h..
jlebar updated this object.
jlebar added subscribers: llvm-commits, silvas.

@chandlerc , should I just land these with post-commit review (after rebasing)? Or, do you have other big patches in flight that you would like me to avoid bitrotting?

("These" being this and D27502.)

chandlerc accepted this revision.Jan 2 2017, 8:18 PM
chandlerc edited edge metadata.

Minor comments below, this patch LGTM with fixes to them.

I'd appreciate doing precommit review on these patches if there is any semantic changes mostly to check and make sure you're not spotting a bug where the old comment should have been correct but wasn't due to bugs or mistakes in the code.

I'm not worried about merge conflicts. The biggest patch i've got in-progress is mostly touching LoopPassManager stuff.

llvm/include/llvm/IR/PassManager.h
133–134

Above you use "Given an analysis's ID", here you use "Given a pointer to its AnalysisKey". I mildly prefer the former wording to the latter, but I'm more interested in consistency.

448–449

Make the DebugLogging comments consistent? And use \p DebugLogging?

This revision is now accepted and ready to land.Jan 2 2017, 8:18 PM
jlebar marked 2 inline comments as done.Jan 4 2017, 4:13 PM

Thank you for the review! I've rebased now, will submit soon.

This revision was automatically updated to reflect the committed changes.