evgeny777 added inline comments.
================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1678
+ // breaks any uses on assumes.
+ if (TypeIdMap.count(TypeId))
+ continue;
----------------
tejohnson wrote:
> 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.
> 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.
I think it's stored in the index in VFuncId structures. Isn't it?
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
