tejohnson added a comment.

In D152741#4419265 <https://reviews.llvm.org/D152741#4419265>, @modimo wrote:

> In D152741#4418831 <https://reviews.llvm.org/D152741#4418831>, @tejohnson 
> wrote:
>
>> I think I understand the motivation, but not sure I agree this is the right 
>> approach - can you simply not pass -flto-unit and -fwhole-program-vtables 
>> for these files?
>
> For our third-party libraries, they're pre-built into native files by GCC so 
> that's unfortunately not an option.

I'm confused - how would you pass this new option then? I was assuming you were 
passing this option to some LLVM built files at the interface of those 
libraries. In which case not passing -flto-unit and -fwhole-program-visibility 
should have a similar effect (suppress the type metadata).

>> Also, isn't this hiding possibly necessary info from WPD that might be 
>> needed for correct class hierarchy analysis affecting other IR modules? I.e. 
>> in the type-metadata-skip-vtable-filepaths.cpp test, what if A was derived 
>> from a struct B, which was also defined/used in another module without this 
>> skipping option. We would lose information about the override of f in A, and 
>> possibly do an incorrect devirtualization elsewhere. It seems like a 
>> dangerous option to provide.
>>
>> It might be better to provide an option that can somehow mark vtables in a 
>> given module as unsafe for devirt, and propagate that info to WPD.
>
> That would nicely side-step mismatched flags. `Public` `vcall_visibility` 
> describes this case but with `--lto-whole-program-visibility` there's no a 
> distinction between `Public` because of deferred vs. `Public` because the 
> type is known unsafe. Thoughts on an `unsafe` `vcall_visibility` to capture 
> the latter notion?

That would be better I think. Maybe "unknown".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152741

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to