[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