aeubanks added inline comments.
================ Comment at: llvm/lib/LTO/LTOBackend.cpp:595 + updatePublicTypeTestCalls(Mod, Conf.HasWholeProgramVisibility); + ---------------- aeubanks wrote: > tejohnson wrote: > > aeubanks wrote: > > > I'm not sure where the best place to put these is > > I don't think that this will work for distributed ThinLTO, where the > > ThinLTO Backends are run via clang, which isn't going to have this config > > variable set (since it is set only during LTO linking). I think something > > may need to be recorded in the index as a flag there at thin link / > > indexing time that can be checked here. > > > > It would then be nice to consolidate this handling into WPD itself, e.g. at > > the start of DevirtModule::run, but unfortunately we won't have a summary > > index for pure regular LTO WPD so I don't think that would work. So in here > > is ok but I would do it early to handle some of the early return cases. > > > > (Please add a distributed ThinLTO test too) > Added a distributed ThinLTO test: > clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll. > > I added `ModuleSummaryIndex::WithWholeProgramVisibility` but I'm not sure > where I'd set it, and that's causing the test to fail. ah, I needed a separate WPV flag in llvm-lto2 to set `llvm::lto::Config`, now everything passes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128955/new/ https://reviews.llvm.org/D128955 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
