Anastasia added a comment. In http://reviews.llvm.org/D20681#450073, @yaxunl wrote:
> In http://reviews.llvm.org/D20681#448443, @Anastasia wrote: > > > Do you think we could add any test for this change? > > > 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! ================ 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. ---------------- 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? http://reviews.llvm.org/D20681 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
