[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Looks good, will likely approve after these 2 questions.



================
Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:157-158
+    // In these cases we should not emit any diagnostic related to misexpect.
+    if (NOps < 3)
+      return;
+
----------------
would an `assert` be more appropriate then?  If cases 1/2/3 above are all highly unlikely, I'd make this an assert rather than early return.


================
Comment at: llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll:79
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "frame-pointer"="none" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { argmemonly nounwind willreturn }
----------------
does the debug info need to be in all of these unit tests?  If you still have the C sources handy, `-g0` is extremely handy in generating more concise IR for unit tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66324/new/

https://reviews.llvm.org/D66324