aeubanks added inline comments.
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:449
+ // We currently only use these for --print-before/after.
+ if (PIC && (!printBeforePasses().empty() || !printAfterPasses().empty())) {
+#define MODULE_PASS(NAME, CREATE_PASS)
\
----------------
jamieschmeiser wrote:
> The tests for the options being empty should be moved to a separate function
> to facilitate expanding this feature for use with other pass instrumentation
> callbacks. Right now, this is buried in here and if another callback wished
> to also use this freature, it might be hard to find. Pulling the test out
> into an aptly named function would make it easier to find and understand.
Makes sense, done.
================
Comment at: llvm/lib/Passes/PassBuilder.cpp:467
+#include "PassRegistry.def"
+#ifndef NDEBUG
+ for (const auto &P : printBeforePasses()) {
----------------
jamieschmeiser wrote:
> This will silently give no tracing in release mode if the pass name does not
> exist (ie the error would not be reported and there would be no output). Is
> this because of efficiency concerns? It will only look for the names that
> are in the option list, which would typically be empty. Rather than walking
> the string map, you could have a local stringset, add the pass names into it
> in the macros above if checking will be done and then use the stringset for
> the error checking. I think this would be more efficient than walking the
> stringmap for producing the errors.
You're right, this should normally be empty so it should fine to always check.
Removed the check for NDEBUG.
When --print-before/after is not empty, any minor slowdown shouldn't matter
since you'd only be using --print-before/after while debugging. So I'd prefer
to keep checking the map which is a bit simpler code-wise.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87216/new/
https://reviews.llvm.org/D87216
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
