[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