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

[Arnt Gulbrandsen via Phabricator via llvm-commits]

arnt marked 3 inline comments as done.
arnt added a comment.

Now with the ones I missed.

Left out are: Some for-loop variable names (AIUI the upper-case rule is widely ignored for those, I join that chorus), and at least one case where I don't see any reason to change type (from std::map) because it's work for insignificant benefit. Maybe I'd do it differently if writing from scratch.



================
Comment at: llvm/include/llvm/Transforms/Utils/ImproveReadingOrder.h:18
+
+using namespace llvm;
+
----------------
jdoerfert wrote:
> Don't open a namespace in a header.
> 
> ```
> namespace llvm {
> ...
> }
> ```
Ugh. I missed this. Coming.


================
Comment at: llvm/include/llvm/Transforms/Utils/ImproveReadingOrder.h:37
+struct ImproveReadingOrder : public PassInfoMixin<ImproveReadingOrder> {
+  ImproveReadingOrder() = default;
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
----------------
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?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60619