[PATCH] D62186: [SimplifyCFG] Fix prof branch_weights MD while removing unreachable switch cases

[Philip Reames via Phabricator via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/two required changes.

The second is that the updater itself should check for the right number of metadata nodes and do nothing if already corrupt (as mentioned in another review thread).



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:4217
       }
-    } else if (auto *SI = dyn_cast<SwitchInst>(TI)) {
+    } else if (auto *S = dyn_cast<SwitchInst>(TI)) {
+      SwitchInstProfBranchWeightsWrapper SI(*S);
----------------
Naming here is non-idiomatic.  Please leave SI for the SwitchInst and use SU or something for the updater class.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:4219
       for (auto i = SI->case_begin(), e = SI->case_end(); i != e;) {
         if (i->getCaseSuccessor() != BB) {
           ++i;
----------------
Not related to your change, but there's no reason a switch can't have several cases leading to the same unreachable block.  A separate change to handle that would be nice if you wanted.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62186