[PATCH] D62123: [NFC] SimplifyCFG prof branch_weighs handling is simplified
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 17 14:23:34 PDT 2019
reames added a comment.
Again, LGTM.
================
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)
----------------
yrouban wrote:
> reames wrote:
> > 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.)
> //HasWeight// is equivalent to the condition SI.State == Invalid.
> SI.State is checked in every method of the wrapper, particularly in the constructor, in SI.removeCase() and in the destructor. Those correspond to the //if (HasWeight)// places.
Yep, I see it now. I must have been looking at a stale checkout or something when I commented originally. Feel free to ignore this thread.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62123/new/
https://reviews.llvm.org/D62123
More information about the llvm-commits
mailing list