[PATCH] D75617: [WPD] Provide a way to prevent function(s) from being devirtualized

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D75617#1907097 <https://reviews.llvm.org/D75617#1907097>, @evgeny777 wrote:

> > This won't work for the index-only WPD.
>
> This can be addressed at the moment, because index based WPD supports only single-impl devirtualization. In this case we have function name in the index.


Meaning we can just check the single impl name down in DevirtIndex::trySingleImplDevirt and block it there? Yeah, that will work.

> 
> 
>> Probably the best way to cover that case is to use the option in ModuleSummaryAnalysis.cpp to block functions from being added to the VTableFuncs in the summary in findFuncPointers.
> 
> I don't think this is enough. If we have call slot { foo, bar }, and want to prevent devirtualization of foo, then entire call slot can't be devirtualized. If `bar` is not used by any other call slot then it won't be devirtualized as well

Right, nevermind, we will actually do incorrect single impl devirts with what I suggested, because we will be missing some of the targets for the slot. I'm not sure I follow your second sentence here, but essentially if the targets for the slot are supposed to be {foo, bar} and if we didn't summarize foo, then we would incorrectly perform a single impl devirt to bar, when we shouldn't have devirtualized at all. Or are you saying something different?



================
Comment at: llvm/test/Transforms/WholeProgramDevirt/virtual-const-prop-check.ll:2
 ; RUN: opt -S -wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt %s 2>&1 | FileCheck %s
+; Skipping vf0i1 is identical to setting public LTO visibility. We don't devirtualize vf0i1 and all other
+; virtual call targets.
----------------
evgeny777 wrote:
> tejohnson wrote:
> > Why does skipping this one in particular cause both vf0i1 and vf1i1 to be skipped?
> > 
> > Also the comment here says that we won't devirtualize vf0i1 "and all other virtual call targets" except that the checking still looks for devirtualizations of vf*i32.
> We have 4 vtables:
> ```
> vtbl1: { vf0i1, vf1i1, vf1i32 }
> vtbl2: { vf1i1, vf0i1, vf2i32 }
> vtbl3: { vf0i1, vf1i1, vf3i32 }
> vtbl4: { vf1i1, vf0i1, vf4i32 }
> ```
> and 3 call slots:
> ```
> CS1: (ByteOffset == 16) : { vf1i32, vf2i32, vf3i32, vf4i32 }
> CS2: (ByteOffset == 8) : { vf1i1, vf0i1 }
> CS3: (ByteOffset == 0) : { vf0i1, vf1i1 }
> ```
> If we want to prevent devirtualization of vf0i1 then both CS2 and CS3 can't be devirtualzied and final devirtualized set is { vf1i32, vf2i32, vf3i32, vf4i32 }
> If we want to prevent devirtualization of  vf1i32 then CS1 can't be devirtualized and final devirtualized set is { vf0i1 vf1i1 }
Ok, nevermind, I was thinking about single impl devirt, wondering how we can devirtualize with multiple targets in the slot, but I see in this case it is because we are doing virtual constant prop.


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

https://reviews.llvm.org/D75617