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

[Arnt Gulbrandsen via Phabricator via llvm-commits]

arnt added a comment.

I pushed an update now. Will this comment submit the unsubmitted drafts?



================
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)                                         \
----------------
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.


================
Comment at: llvm/lib/Transforms/Utils/ImproveReadingOrder.cpp:134
+
+    while(current && processed.count(current)) {
+      if(!toBeDone.empty())
----------------
MaskRay wrote:
> In some pessimistic cases, `preferredNextBlock` is of `O(|input|^2)` and this while loop is cubic..
> 
> Do you want to rewrite the loop to use a worklist?
> 
> ```
> if (condition) {
>   complex   /////// I believe the current algorithm only reorders the 'then' part
> } else {
>   complex    /////// this is not touched
> }
> ```
> 
The while loop exits as soon as preferredNextBlock() returns non-null, so it's quadratic, not cubic.

I'm not sure whether a rewrite for speed is worthwhile. It'll help if a function has very many dead blocks, exception handlers or targets for a single switch, but it's not clear to me whether it'll ever make such a function readable. Sucking quickly instead of slowly isn't worth much effort.

On the other hand, if a function has, say, a thousand dead blocks, then one may have a particularly strong reason to inspect that function, and printing it shouldn't take thirty minutes.

So I'm on the fence.

I rewrote. You pointed me to SetVector and SetVector wants to be used ;) The new version's weak point is a thousands-wide switch where half the branches fall through to the other half, and the fallen-through-to branches contain many basic blocks and loops and so on. I'll upload a new revision later today.


================
Comment at: llvm/test/Transforms/Util/improve-reading-order.ll:1
+; RUN: opt -passes improve-reading-order < %s | FileCheck %s
+
----------------
MaskRay wrote:
> Missing `-S`
Ooops, fixed. Why did the tests pass without? It seems I've been running check-llvm-unit instead of check-llvm.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60619