[PATCH] D73242: [WPD/LowerTypeTests] Delay lowering/removal of type tests until after ICP

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson marked an inline comment as done.
tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+    // breaks any uses on assumes.
+    if (TypeIdMap.count(TypeId))
+      continue;
----------------
evgeny777 wrote:
> tejohnson wrote:
> > evgeny777 wrote:
> > > I don't think, I understand this.
> > > It looks like that if (a) we have type.test in the module and (b) we don't have vtable definition with corresponding type metadata in the same module then we'll remove type.test/assume sequence before even getting to ICP. This seems strange in the context of previous discussion because virtual function may be called in different module from one where vtable is defined, like so:
> > > ```
> > > struct A { virtual int foo() { return 42; } };
> > > 
> > > // calls pA->foo
> > > extern int callFoo(A *pA);
> > > int main() { A a; return callFoo(&a); }
> > > ```
> > > 
> > We will only be able to do ICP this way if we have the target vtable in the module (similar to how we currently can only do ICP if the target function is in the module). I anticipate doing something similar for vtable defs as to what we do for virtual functions, where a fake call edge is added to the summary based on the value profile results to ensure they are imported before the LTO backend ICP.
> > We will only be able to do ICP this way if we have the target vtable in the module (similar to how we currently can only do ICP if the target function is in the module).
> 
> I was thinking of slightly different approach: it seems you have required type information in combined index together with type name in the module, so you probably can add external declarations of required vtables (this may require promotion) to the module in the ICP pass and run ICP over them. Do you think this can work?
Possibly, but we'd still need to have type metadata on those vtable declarations, because the type metadata provides the address point offset which is needed in the comparison sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73242