[PATCH] D62123: [NFC] SimplifyCFG prof branch_weighs handling is simplified

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 12 15:23:50 PDT 2019


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

LGTM w/two required changes.  (If you want to just make the changes, you can submit without further review. )



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:867
 
-    SwitchInst *SI = cast<SwitchInst>(TI);
+    SwitchInstProfBranchWeightsWrapper SI = *cast<SwitchInst>(TI);
     // Okay, TI has cases that are statically dead, prune them away.
----------------
SwitchInstProfBranchWeightsWrapper SI(*cast<SwitchInst>(TI));

(i.e. avoid a default init and copy construct)


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:879
-    MDNode *MD = SI->getMetadata(LLVMContext::MD_prof);
-    bool HasWeight = MD && (MD->getNumOperands() == 2 + SI->getNumCases());
-    if (HasWeight)
----------------
This HasWeight check doesn't appear to have a corresponding bit of code inside the wrapper class.  In particular, this means that the old code handles malformed metadata and the new one does not.

I know you want to disallow this case, but for the moment, as a practical migration, please add the bailout inside the wrapper.  That would make this change obviously NFC (which it isn't right now.)


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

https://reviews.llvm.org/D62123





More information about the llvm-commits mailing list