hassnaaHamdi wrote:

> > Thanks for your patience - I needed to think about the best option to use 
> > for the new functionality, and what this should look like longer term. I 
> > found one used by gcc for this purpose, -fdevirtualize-speculatively, see 
> > info in comments below. I have some comments sprinkled throughout but here 
> > is a summary along with a suggestion for splitting the patch and how to 
> > structure.
> > Long-term, ideally we can always do speculative devirtualization (probably 
> > under the above option), for any public visibility vtable, and 
> > non-speculative devirt for any hidden ones (which might include those that 
> > were originally public under LTO and whole program visibility options that 
> > separately convert the visibility). The eventually state should probably be 
> > something like:
> > ```
> > +------------------------------------+------------------------------------+------------------------------------+
> > | LTO+WPV                            | -fdevirtualize-speculatively off   | 
> > -fdevirtualize-speculatively on    |
> > +------------------------------------+------------------------------------+------------------------------------+
> > | off                                | none                               | 
> > spec devirt for all vis (or just   |
> > |                                    |                                    | 
> > public and non-spec devirt for     |
> > |                                    |                                    | 
> > hidden vis?)                       |
> > +------------------------------------+------------------------------------+------------------------------------+
> > | on                                 | non-spec devirt for hidden vis     | 
> > non-spec devirt for hidden vis and |
> > |                                    |                                    | 
> > spec devirt for public vis         |
> > +------------------------------------+------------------------------------+------------------------------------+
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > (hopefully that renders ok)
> > I'd suggest staging the changes like this:
> > PR 1: LLVM changes only, using a cl::opt that generically enables 
> > speculative devirt (and TODOs noting that the code should eventually 
> > support spec devirt for any public visibility vtables longer term) PR 2: 
> > clang changes to add new option and LLVM pipeline changes to enable the new 
> > optimization (probably WPV+LTO overriding it for now with TODO noting 
> > longer term plan). PR 3 (longer term): conditionally doing spec devirt for 
> > public visibility vtables
> 
> PR1 is ready: #159048

PR2 is ready: https://github.com/llvm/llvm-project/pull/159685 

https://github.com/llvm/llvm-project/pull/145031
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to