[PATCH] D39111: Extensible LLVM RTTI

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D39111#1929413 <https://reviews.llvm.org/D39111#1929413>, @lhames wrote:

> In D39111#1928251 <https://reviews.llvm.org/D39111#1928251>, @dblaikie wrote:
>
> > In D39111#1925642 <https://reviews.llvm.org/D39111#1925642>, @lhames wrote:
> >
> > > @dblaikie, @chandlerc -- Any further thoughts on this one?
> >
> >
> > I'd like Chandler to come back around on this, given the reservations he expressed, but not sure he'll get to it - so might have to just figure it out ourselves. I share some of those reservations (that'd be easy to misuse this & have code that doesn't work on Windows - people won't read the documentation & might write the broken template situation), but maybe less concerned than Chandler was.
>
>
> Were there reservations about windows related to this patch?


I think this bit amounts to "reservations because of Windows":

"Sadly, [using a CRTP base to provide the ID] will (I suspect) run into the non-conforming implementation of static data members of class templates on MSVC between DLLs. Specifically, the address of ID will be different between two DLLs. =[

I'm actually a bit hesitant with this entire thing as a consequence. We need to really clearly document the hard constraints on how this CRTP base is used as a consequence of this (you cannot use this as part of class templates unless you explicitly instantiate all specializations of them)."

> This certainly came up with the original design of the RTTI for Error: ErrorInfo originally defined the ID member for you, which Chandler pointed out is unsafe on Windows. We solved that in the ErrorInfo case by requiring you to define the ID member in your own class, and RTTIExtends takes the same approach.

The existence of the ErrorInfo doing something similar (could it be ported to use this infrastructure, then?) does help justify this a bit, but I appreciate the concerns of generalizing this further.

>>> I'd like to get this in as we have some use cases in the JIT (e.g. querying and down casting to user defined MaterializationUnit types when dispatching JIT work). At the time these were future use cases, now they're current ones. :)
>> 
>> Could you sketch up a user/use case to demonstrate the use (maybe a Kaleidoscope example?)?
> 
> Here are some simplified versions of the two use cases I want to use straight away:
> 
> In LLJIT set up we want the object linking layer type to be detectable to simplify setup code. E.g.
> 
>   auto J = ExitOnErr(LLJITBuilder().create());
>   if (auto *RTDyldLinkLayer = dyn_cast<RTDyldObjectLinkingLayer>(J->getObjLinkingLayer()))
>     RTDyldLinkLayer->setProcessAllSections(...);
>   else if (auto *ObjLinkLayer = dyn_cast<ObjectLinkingLayer>(J->getObjLinkingLayer()))
>     installObjLinkingLayerPlugins(*ObjLinkingLayer);

This currently would vary by platform & some other things, such that it's a dynamic choice you want to check for?

But for a user-defined ObjLinkingLayer - the user would know which type they had used and could static_cast instead, yes?

I guess maybe that raises a thought: Is this about having an open hierarchy, where you can dyn_cast to specific pre-provided types, or necessarily to support dyn_casting to user-provided types too? Would a solution that only allows dyn_cast between LLVM-provided types be sufficient for your use cases? (yeah, I know it'd be weird & I could see not wanting to do that because of the awkward asymmetry, but trying to understand the problem space a bit further)

> In the dispatcher we want to be able to inspect the dynamic type of MU to decide how to dispatch the work:
> 
>   ES.setDispatchMaterialization([](JITDylib &JD, std::unique_ptr<MaterializationUnit> MU) {
>     if (isa<SplattingMaterializationUnit>(*MU))
>       MU->doMaterialize(JD); // Materialize splatting MUs in-place.
>     else if (auto &IRMU = dyn_cast<IRMaterializationUnit>(*MU))
>       dispatchIRMU(std::move(MU)); // Analyze IR and decide how to dispatch
>     else
>       CompileThreads->async([MU=std::move(MU), &JD]() {
>         MU->doMaterialization(JD); // Otherwise default to materializing on background thread
>       });
>   });

Same sort of question - I guess the LLVM version of this code would be closed, but users would provide their own kinds of MUs - to support those users using both their own custom ones and standard ones, do the users control the MUs, such that they could test for all the types of MUs they know they are using in their JIT. Yeah, that'd be a bit awkward to be "is it MyCustomMU3" ends up being "is it not an IRMU, not a <other LLVM-provided MU>, etc... then it must be a MyCustomMU of some kind, so then do a different pseudo RTTI test".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D39111