[PATCH] D65490: [SimplifyCFG] Mark missed Changed to true.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:4192
           EraseTerminatorAndDCECond(BI);
+          Changed = true;
         } else if (BI->getSuccessor(1) == BB) {
----------------
sanjoy wrote:
> You could sink down the `Changed = true;` outside the if/else.
Right, but this is non-obvious because of the `else if` below (instead of `else`). Unless one notices that this iterates over BB's predecessors, then they may think BB may not be either the 0 or 1 successor.
Let me send the clean-up patch for the above and for this separately, since we can also sink the call to `EraseTerminatorAndDCECond(BI);` here.


================
Comment at: test/Transforms/SimplifyCFG/invalidate-dom.ll:16
+  store i32 %h, i32* %h.addr, align 4
+  %0 = load i32, i32* %h.addr, align 4
+  switch i32 %0, label %sw.default [
----------------
sanjoy wrote:
> You can use `opt -metarenamer` to give names to all these instructions.  That makes the test a bit more resilient to changes.
SG, but I renamed just the instructions manually. The metarenamer renames everything and I for one get more confused when everything is named foo, bar etc :).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65490