paulkirth marked 5 inline comments as done. paulkirth added inline comments.
================ Comment at: llvm/docs/MisExpect.rst:54 + +.. option:: -pgo-warn-misexpect + ---------------- tejohnson wrote: > paulkirth wrote: > > tejohnson wrote: > > > Looking at the code, the -pgo-warn-misexpect seems to be useful for > > > internal LLVM testing via 'opt', to mimic -Wmisexpect, so it probably > > > should be documented as such. > > Yes, its useful to test llvm, but shouldn't `opt` be usable in the same > > way? To me it seems useful for opt to support the warning directly too, but > > I'm happy to defer here if you think that's confusing or shouldn't be the > > case. > Oh I'm not disagreeing with having the internal option and using it for opt, > that's very useful. My comment was in the context of not having the > user-facing clang documentation with the clang driver level option (which you > since added). I was just suggesting you add a note that this option is an > alternative to the clang option for use when e.g. testing via opt. Since > there is now separate clang documentation I think it is less important now. Thanks for the clarification. ================ Comment at: llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll:3 + +; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s + ---------------- tejohnson wrote: > tejohnson wrote: > > The old PM is gone in the optimization pipeline, so these old PM lines in > > this and other tests and the comment about the new PM can go away. > I see you removed the NewPM-style invocations and kept the legacy ones - > afaict opt will now convert the legacy style pass invocations to the new PM, > but I think it is preferable to use the New PM style invocations. yes, I misread your earlier comment. I have restored the New PM style opt invocations, and removed the legacy PM ones. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
