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

Reply via email to