hoy added a comment. In D93656#2469504 <https://reviews.llvm.org/D93656#2469504>, @dblaikie wrote:
> In D93656#2469485 <https://reviews.llvm.org/D93656#2469485>, @hoy wrote: > >> In D93656#2468841 <https://reviews.llvm.org/D93656#2468841>, @aeubanks wrote: >> >>> In D93656#2468834 <https://reviews.llvm.org/D93656#2468834>, @hoy wrote: >>> >>>> In D93656#2468821 <https://reviews.llvm.org/D93656#2468821>, @aeubanks >>>> wrote: >>>> >>>>> Also it looks like this is doing 2 different things, the moving of things >>>>> from Clang to LLVM's PassBuilder, and separately the change to the pass >>>>> itself. Can these be separated? >>>> >>>> I'm not sure about a good way to separate them. There are Clang tests that >>>> may fail with removing the pass from clang while not adding it >>>> correspondingly in llvm. Adding the pass in llvm while not removing it >>>> from Clang may cause the pass to run twice which may also fail the Clang >>>> tests. What do you think? >>> >>> I mean keep that in one change, but separate out the change to >>> llvm/lib/Transforms/Utils/UniqueInternalLinkageNames.cpp and >>> DebugInfoMetadata.h. >> >> I see, thanks for the clarification. >> >> In D93656#2468698 <https://reviews.llvm.org/D93656#2468698>, @dblaikie wrote: >> >>> This should have a LLVM test coverage for the LLVM changes. (I realize >>> they're also tested by the Clang test, because there's no way to test >>> Clang's pass manager creation short of testing the effect of running the >>> pass manager (hmm - /maybe/ there's a way to dump the pass pipeline? In >>> which case that's how Clang should be tested, just testing that it creates >>> the right pipeline, not that that pipeline does any particular thing)) >> >> Added an IR test. There is the llvm switch `-debug-pass=` that can dump the >> pass pipeline. I'm not aware of a clang switch that can do that. > > Seems some clang tests use something like that, eg: > > clang/test/CodeGen/thinlto-debug-pm.c:// RUN: %clang_cc1 -triple > x86_64-unknown-linux-gnu -emit-obj -O0 -o %t2.o -x ir %t.o > -fthinlto-index=%t.thinlto.bc -fno-experimental-new-pass-manager -mllvm > -debug-pass=Structure 2>&1 | FileCheck %s --check-prefix=O0-OLDPM Thanks for pointing that out. It seems that only works with the legacy pass manager when `-emit-llvm` is specified. With a quick search in the clang regression tests it looks like the pipeline dumps are rarely checked there. Such checks are done quite a bit in llvm tests though. Does the current clang test look okay to you, or do you prefer checking the pipeline dump there? Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93656/new/ https://reviews.llvm.org/D93656 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
