[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a subscriber: fhahn.
tejohnson added a comment.

In D128955#3674787 <https://reviews.llvm.org/D128955#3674787>, @aeubanks wrote:

> random question, if the old API is "legacy", are there any plans to remove it?

@fhahn started to work on this at some point (see https://bugs.llvm.org/show_bug.cgi?id=41541), but I'm not sure of the status. It is used by ld64 and I believe the sony toolchain too.



================
Comment at: lld/test/ELF/lto/update_public_type_test.ll:5
+
+; RUN: opt --thinlto-bc -o %t.o %s
+; RUN: ld.lld %t.o -o %t2.o --save-temps
----------------
Just realized there isn't any testing of the regular LTO handling with the new LTO API. Can you extend this test to check regular LTO as well?


================
Comment at: llvm/test/LTO/X86/public-type-test.ll:3
+
+; RUN: opt -module-summary %s -o %t.bc
+; RUN: llvm-lto --thinlto-action=run -exported-symbol=_main %t.bc --thinlto-save-temps=%t2
----------------
aeubanks wrote:
> tejohnson wrote:
> > The new version switched this from a regular LTO to ThinLTO test. I think we need both. Can you move the new version into llvm/test/ThinLTO/X86 and make this version back to testing the regular LTO old API handling, but with checks?
> done, had to add a new flag to dump IR right before optimizations
I see, looks like llvm-lto has a thinlto-save-temps that is wired up to dump the same outputs as the new LTO API's save temps, but regular LTO didn't have the same support. I'd go ahead and use the same naming convention (0.preopt.bc) for compatibility with the new LTO API, in case that save temps handling is ever generalized to support both thin and regular LTO.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128955/new/

https://reviews.llvm.org/D128955