[PATCH] D60619: New pass to produce more easily-read IR.

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 8 16:17:33 PDT 2019


jdoerfert added a comment.

I haven't had time to read through the explanation that is to become a comment yet.



================
Comment at: llvm/include/llvm/Transforms/Utils/ImproveReadingOrder.h:37
+struct ImproveReadingOrder : public PassInfoMixin<ImproveReadingOrder> {
+  ImproveReadingOrder() = default;
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
----------------
arnt wrote:
> jdoerfert wrote:
> > Most "new PM" transformations only expose the run method. See the `preferredNextBlock` definition comment.
> I didn't notice that from the ones I skimmed... is this a rule, and if so, what motivates it?
Afaik, not a hard rule, just something I feel is being adapted as the norm.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:1700
 
-  // Finally expand the basic registered passes from the .inc file.
+  // Finally expand the basic registered passes from PassRegistry.def.
 #define MODULE_PASS(NAME, CREATE_PASS)                                         \
----------------
arnt wrote:
> MaskRay wrote:
> > Good catch! This is irrelevant to this revision. You may commit it separately.
> Can do, but I'll do it at the same time, OK? Haven't figured out how to push from my git monorepo to the svn source of truth yet.
`git llvm push HEAD~2` will push the last two commits. `-n` is for dryrun. 


================
Comment at: llvm/lib/Transforms/Utils/ImproveReadingOrder.cpp:116
+      std::map<std::pair<uint, uint>, BasicBlock *> Ordered;
+      uint n = 0;
+      while (n < Terminator->getNumSuccessors()) {
----------------
Please do not use `uint`, I'm with him: https://stackoverflow.com/a/5678057


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60619/new/

https://reviews.llvm.org/D60619





More information about the llvm-commits mailing list