nickdesaulniers added inline comments.
================ Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150 + // Operand 0 is a string tag "branch_weights" + if (MDString *Tag = cast<MDString>(MD->getOperand(0))) { + unsigned NOps = MD->getNumOperands(); ---------------- paulkirth wrote: > nickdesaulniers wrote: > > Sorry if I'm going back and forth on this, but it may make sense to check > > the number of operands first, before accessing any. > No problem. I've reintroduced the check. I was under the impression that > metadata could not lack a 0th element, but even in that case I should have > checked for additional elements. > > There should always be at least 2 branch weights, otherwise branch weights > are not necessary, and we should never end up here. > > Should I add a comment saying that? Sorry, I meant "check the lower bound of the operands first, ..." as `NOps` might be `0` which would cause `MD->getOperand(0)` to fail, and `RealWeights` to be size `-1`. `if (Nops < 1 || Nops > 3) return;` I think a comment would be helpful, too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits