[mlir] ExecutionEngine needs special handling for COFF binaries

[mehdi_amini]

mlir/lib/ExecutionEngine/ExecutionEngine.cpp

262

Please document all this.

I'm also curious why this isn't handled by Orc directly?

[kernhanda]

Adding comments

[kernhanda]

mlir/lib/ExecutionEngine/ExecutionEngine.cpp

262

Added comments. Can't tell you why it isn't handled by Orc directly, but LLJIT seems to be doing the same thing (noted in the documentation comment).

[mehdi_amini]

@lhames any opinion on whether Orc should handle this for the user? Seems like platform specific logic that every client would have to reimplement?

[kernhanda]

ping :)

[mehdi_amini]

I think Lang is on vacation right now, can you file a bug to Lang to look into this and add a TODO referencing the bug here?

[lhames]

In D97141#2583436, @mehdi_amini wrote:

I think Lang is on vacation right now, can you file a bug to Lang to look into this and add a TODO referencing the bug here?

Ish. Every second day is a vacation day for the next two weeks -- I head back to the US then, so enjoying Australia while I can. In the mean time replies will be delayed sightly.

In D97141#2579033, @mehdi_amini wrote:

@lhames any opinion on whether Orc should handle this for the user? Seems like platform specific logic that every client would have to reimplement?

In an ideal world we shouldn't need them, but our COFF support isn't up to scratch yet. Turning these flags on by default for COFF is probably the right choice. In the long term a COFF JITLink implementation (and proper COMDAT support) is the right answer.

[kernhanda]

Thanks @lhames and @mehdi_amini ! I'll be landing this patch now

[mehdi_amini]

Turning these flags on by default for COFF is probably the right choice.

I thinks this still deserves a bug?

[kernhanda]

In D97141#2583687, @mehdi_amini wrote:

Turning these flags on by default for COFF is probably the right choice.

I thinks this still deserves a bug?

Created https://bugs.llvm.org/show_bug.cgi?id=49348. Let me know if there's anything else I can do.

mlir/lib/ExecutionEngine/ExecutionEngine.cpp

auto objectLayer = std::make_unique<RTDyldObjectLinkingLayer>(
         session, []() { return std::make_unique<SectionMemoryManager>(); });
 
     // Register JIT event listeners if they are enabled.
     if (engine->gdbListener)
       objectLayer->registerJITEventListener(*engine->gdbListener);
     if (engine->perfListener)
       objectLayer->registerJITEventListener(*engine->perfListener);
 
+    // COFF format binaries (Windows) need special handling to deal with
+    // exported symbol visibility.
+    // cf llvm/lib/ExecutionEngine/Orc/LLJIT.cpp LLJIT::createObjectLinkingLayer
+    llvm::Triple targetTriple(llvm::Twine(llvmModule->getTargetTriple()));
+    if (targetTriple.isOSBinFormatCOFF()) {
+      objectLayer->setOverrideObjectFlagsWithResponsibilityFlags(true);
+      objectLayer->setAutoClaimResponsibilityForObjectSymbols(true);
+    }
+
     // Resolve symbols from shared libraries.
     for (auto libPath : sharedLibPaths) {
       auto mb = llvm::MemoryBuffer::getFile(libPath);
       if (!mb) {
         errs() << "Failed to create MemoryBuffer for: " << libPath
                << "\nError: " << mb.getError().message() << "\n";
         continue;
       }