[PATCH] D155659: [WPD][LLD] Add option to validate RTTI is enabled on all native types and prevent devirtualization on types with native RTTI

[Di Mo via Phabricator via llvm-commits]

modimo added a comment.

Thanks for the review!



================
Comment at: lld/test/ELF/lto/devirt_validate_vtable_typeinfos.ll:6
+; RUN: opt --thinlto-bc --thinlto-split-lto-unit -o %t1_hybrid.o %s
+; RUN: cp %s %t1_regular.ll
+; RUN: echo '!llvm.module.flags = !{!12}' >> %t1_regular.ll
----------------
tejohnson wrote:
> modimo wrote:
> > Appending module flags so RegularLTO correctly generates it's summary without `typeidCompatibleVTable` means the test can be re-used. However I think duplicating the tests is reasonable as well and could be cleaner, WDYT?
> Do we need these module flags for correct operation of this test (ditto for the similar no_rtti one later)? If not, then probably don't bother adding in these tests (I think these may only be needed in practice for the hybrid testing). If they are now needed for correct operation of the regular LTO testing, then I am ok with the approach here as I think it is probably better to reduce duplication of nearly identical IR tests (and I see this approach used in other tests too).
>Do we need these module flags for correct operation of this test (ditto for the similar no_rtti one later)?

Yeah, to trigger summary generation but on the RegularLTO pipeline requires these module flags.

>If they are now needed for correct operation of the regular LTO testing, then I am ok with the approach here as I think it is probably better to reduce duplication of nearly identical IR tests (and I see this approach used in other tests too).

Sounds good, I'll leave it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155659