This is an archive of the discontinued LLVM Phabricator instance.

[mlir][CRunnerUtils] Use explicit execution engine symbol registration.
ClosedPublic

Authored by ingomueller-net on Jun 20 2023, 7:38 AM.

Details

Summary

As a follow up of https://reviews.llvm.org/D153250, this path uses the
explicit symbol registration mechanism of the execution engine in the
CRunnerUtils library.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2023, 7:38 AM
ingomueller-net requested review of this revision.Jun 20 2023, 7:38 AM
mehdi_amini accepted this revision.Jun 20 2023, 8:42 AM
This revision is now accepted and ready to land.Jun 20 2023, 8:42 AM

Marking the symbols that aren't exported explicitly as hidden. I'll wait for CI to succeed and then land, assuming that this change is OK. Otherwise, we'll revert.

CI failed because, in the Windows builds, two symbols implemented in the mlir_float16_utils libraries and exported through the init/destroy mechanism in the mlir_c_runner_utils libraries could not be found by the linker. I can only suspect (because I don't have a Windows machine) that this is because these two symbols were not explicitly exported in the mlir_float16_utils library, so I am now exporting those explicitly in that lib. I'll see if I am right in a bit when CI finishes...

(Rebasing to resolve unrelated build error.)

antmo added a subscriber: antmo.Jun 21 2023, 8:40 AM

Hi @ingomueller-net,
I suspect this patch broke two bots:

Is that correct? Could you please take a look?

ingomueller-net added a comment.EditedJun 21 2023, 11:16 AM

@antmo: thanks for investigating! @mehdi_amini: thanks for reverting!

I found the problem: SparseRuntime.cpp is part of the mlir_c_runner_utils lib and I did not export the symbols of that file or change their visibility. While I can simply do that, I wonder whether they shouldn't be in their own lib? Opinions?