If OptNoneInstrumentation prints it instead, 'Skipping pass' will print for even required passes.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/lib/Passes/StandardInstrumentations.cpp | ||
---|---|---|
314 | We probabaly don't need to check adaptor&pass manager here because they are never skipped. | |
317 | It would be more clear to also print the IR name as registerBeforeNonSkippedPassCallback did (using unwrapAndPrint) . | |
llvm/test/Feature/optnone-opt.ll | ||
96 | Yeah, we don't have a required user pass yet to test. And I would rather not special-casing Verifer pass for this. How about, instead of doing this, extending NPM-LOOP check with IR names so we know the new code path is covered. We could defer the required passes testing for the future. There are some candidate machine passes that could be marked required. |
llvm/lib/Passes/StandardInstrumentations.cpp | ||
---|---|---|
314 | I could imagine them being skipped in the future. For example, an adaptor could check if the pass it's wrapping is required or not. So I think keeping this is fine. | |
llvm/test/Feature/optnone-opt.ll | ||
96 | The point of this patch was that "Skipping pass" was printed when it shouldn't have been. Adding checks to NPM-LOOP doesn't really test that, CHECK-NOT: Skipping pass is really the important thing, and for that we need some required pass. |
llvm/lib/Passes/StandardInstrumentations.cpp | ||
---|---|---|
314 | If it is for the future and you're worried that it may not work as expected. I would suggest adding an assert to check the pass could not be special pass yet. Doing so we don't need to add a test for this and worry about the use cases for it. | |
llvm/test/Feature/optnone-opt.ll | ||
96 | Then let's check pass managers or adaptors as the required pass (checking there are no Skipping pass for them or using CHECK-NEXT). In terms of code path, they make no difference from the required user pass. Making Verifier pass required could be a separate patch because we need to discuss the use cases for that. |
llvm/test/Feature/optnone-opt.ll | ||
---|---|---|
96 | Sounds good, done. |
We probabaly don't need to check adaptor&pass manager here because they are never skipped.