[PATCH] D20681: Add target-specific pre-linking passes to Clang

[Anastasia Stulova via cfe-commits]

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