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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
