[PATCH] D48369: [CodeGen] Make block removal order deterministic in CodeGenPrepare

[David Stenberg via Phabricator via llvm-commits]

dstenb added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:468
+    // are removed.
+    SmallSetVector<BasicBlock*, 8> WorkList;
     for (BasicBlock &BB : F) {
----------------
fhahn wrote:
> IIUC  for each BB we look at the successor and only add a successor to the worklist, if it has exactly one predecessor, right? Doesn't that mean we only ever add each BB once and there wouldn't be a need for a set?
At least with how successors are handled now, we need it to be a set due to switch instructions where multiple cases share the same successor (for example `%bb857.i` in `test/CodeGen/X86/2010-01-11-ExtraPHIArg.ll`).


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:483-484
     while (!WorkList.empty()) {
       BasicBlock *BB = *WorkList.begin();
-      WorkList.erase(BB);
+      WorkList.remove(BB);
       SmallVector<BasicBlock*, 2> Successors(succ_begin(BB), succ_end(BB));
----------------
dexonsmith wrote:
> This call to `remove` is linear.  Popping from the back would be amortized constant time:
> ```
> BasicBlock *BB = WorkList.pop_back_val();
> ```
> 
Oh, sorry! Fixed in the next revision.


Repository:
  rL LLVM

https://reviews.llvm.org/D48369