yaxunl marked 2 inline comments as done.
yaxunl added a comment.
> > We have prelinking passes in amdgpu backend but it requires the llvm change
> > to be committed first. We can add a test for this after that.
>
>
> Sure. Could you subscribe me to the relevant backend reviews if possible
> please? Thanks!
Sure, will do when we review such a pass.
================
Comment at: lib/CodeGen/CodeGenAction.cpp:169
@@ +168,3 @@
+ std::function<bool(llvm::Module*)>
+ LinkCallBack = [=](llvm::Module *M)->bool {
+ // Link LinkModule into this module if present, preserving its
validity.
----------------
Anastasia wrote:
> yaxunl wrote:
> > Anastasia wrote:
> > > Is there any reason for having this as a callback now?
> > >
> > > Could we just add a call to prelink passes here above instead without
> > > modifying much the original flow?
> > EmitBackendOutput does not set its own diagnostic handler. When linking
> > error happens, the diagnostic handler of BackendConsumer is used, which
> > requires the member variable CurLinkModule of BackendConsumer to be set to
> > current module to be linked to emit correct error msg. Therefore the
> > linking step of EmitBackendOutput needs to update a member of
> > BackendConsumer, a lambda function can achieve that with minimal change to
> > other parts of the code.
> >
> > An alternative implementation can do without the lambda function, but needs
> > to
> >
> > # define a diagnostic handler for EmitBackendOutput
> > # at the begining of EmitBackendOutput, save the old diagnostic handler
> > and set the new one
> > # at the end of EmitBackendOutput, recover the old diagnostic handler
> > # define a helper class for diagnostic handler for EmitBackendOutput to
> > retain the states needed for emitting diagnostic msgs
> > # move or copy the diagnostic handling required by EmitBackendOutput from
> > the diagnostic handler of BackendConsumer to the helper class of diagnostic
> > handler for EmitBackendOutput
> >
> > Do we want to take this approach?
> >
> I see, seems complicated indeed. Would returning the module into
> CurLinkModule be possible instead?
We can replace the lambda function argument with two arguments: the modules to
be linked with, and a pointer to CurLinkModule. Then CurLinkModule can be
updated during linking.
There may be a more elegant way. I will see if I can add a new function for
performing pre-linking passes and call it from HandleTranslationUnit. Then we
can keep the linking stage in HandleTranslationUnit unchanged.
http://reviews.llvm.org/D20681
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
