Page MenuHomePhabricator

[flang] Check there's no dependency on C++ libs
ClosedPublic

Authored by rovka on Jun 15 2021, 4:06 AM.

Details

Summary

Add a test to make sure the flang runtime doesn't pull in the C++
runtime libraries.

This is achieved by adding a C file that calls some functions from the
runtime (currently only CpuTime, but we should probably add anything
complicated enough, e.g. IO-related things). We force the C compiler to
use -std=c90 to make sure it's really in C mode (we don't really care
which version of the standard, this one is probably more widely
available). We only enable this test if CMAKE_C_COMPILER is set to
something (which is probably always true in practice).

Diff Detail

Event Timeline

rovka created this revision.Jun 15 2021, 4:06 AM
rovka requested review of this revision.Jun 15 2021, 4:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2021, 4:06 AM
jeanPerier accepted this revision.Jun 15 2021, 8:04 AM

Looks like a good idea to me to have this test, thanks !

flang/test/lit.cfg.py
85

I wonder if you should also add libFortranDecimal.a to this, libFortranRuntime.a depends upon it. The dependency is not required for your test, so maybe this could also be done when adding an I/O runtime call use that actually depends on it.

This revision is now accepted and ready to land.Jun 15 2021, 8:04 AM

I would like to have some automated check in the build that would catch an inadvertent binary dependency, but this scheme seems to admit false negative results when the linkage is static.

I have an extra step in my build that scrapes the runtime libraries' binaries' unsatisfied external symbols in search of mangled C++ names, but that's obviously not portable.

When the llvm tree can build an end-to-end Fortran compiler, if we're allowed to have end-to-end Fortran compilation tests, they'll be better checks for C++-free linking.

rovka added a comment.Jun 16 2021, 1:02 AM

I would like to have some automated check in the build that would catch an inadvertent binary dependency, but this scheme seems to admit false negative results when the linkage is static.

This just tests that CpuTime doesn't bring in any C++ symbols. If we added calls to all the runtime functions here, we could be reasonably confident that nothing in the runtime lib brings in C++ symbols, right? So there shouldn't be any false negatives unless the C compiler does something funny. I suppose we could avoid that if we called the linker directly? I'm not sure off the top of my head how complicated it would be to do that in a portable way.

I have an extra step in my build that scrapes the runtime libraries' binaries' unsatisfied external symbols in search of mangled C++ names, but that's obviously not portable.

When the llvm tree can build an end-to-end Fortran compiler, if we're allowed to have end-to-end Fortran compilation tests, they'll be better checks for C++-free linking.

We can definitely have end-to-end fortran tests. In the worst case, they'll have to live in the test-suite :) But I'm thinking when flang can handle linking we could just replace this file with the fortran equivalent and call flang instead of cc.

flang/test/lit.cfg.py
85

Yeah, I think it's better to add it when there's a need for it. It will be easy to know because the test will fail then :)

This revision was automatically updated to reflect the committed changes.
awarzynski added a subscriber: awarzynski.EditedJun 16 2021, 5:00 AM

Hi @rovka , could you take a look: https://lab.llvm.org/buildbot/#/builders/160/builds/2726? Looks like it should just be a matter of replacing // with /* in the test file. Thank you!

EDIT:
Ive just noticed https://reviews.llvm.org/rG0ad051b5fc22b4e7334635b2bdc54eca8ee7c4c4, thanks for the fix and apologies for the noise!

Hi Andrzej,

I already fixed that and another issue as of 0ad051b5fc22 (It hasn't been
picked up by that builder yet).

Cheers,
Diana