[PATCH] D65490: [SimplifyCFG] Mark missed Changed to true.
Alina Sbirlea via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 1 11:30:22 PDT 2019
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
More information about the llvm-commits
mailing list