Page MenuHomePhabricator

Dropped non-supoorted "--no-as-needed" flag from OMPT tests for macOS
ClosedPublic

Authored by simoatze on Jul 3 2018, 11:02 AM.

Details

Summary

The flag "--no-as-needed" is not recognized by the linker on macOS making the following tests fail:

  • ompt/loadtool/tool_available/tool_available.c
  • ompt/loadtool/tool_not_available/tool_not_available.c

This patch removes this flag for macOS and adds it only for Linux and Windows.
I tested it on Ubuntu 16.04 and macOS HighSierra, with Clang/LLVM 6.0.1 and OpenMP trunk.

This solution was also discussed in the OpenMP-dev mailing list.

Diff Detail

Repository
rL LLVM

Event Timeline

simoatze created this revision.Jul 3 2018, 11:02 AM
protze.joachim requested changes to this revision.Jul 3 2018, 12:47 PM

As discussed on the mailing list, the flag should only be dropped on Mac OS. My pragmatic solution would be:

if config.operating_system == 'Darwin':
    config.substitutions.append(("--Wl,--no-as-needed", ""))

Not sure what @Hahnfeld thinks about this solution :)

The cleaner solution would probably be to define a %no-as-needed-flag substitution.

This revision now requires changes to proceed.Jul 3 2018, 12:47 PM

As discussed on the mailing list, the flag should only be dropped on Mac OS. My pragmatic solution would be:

if config.operating_system == 'Darwin':
    config.substitutions.append(("--Wl,--no-as-needed", ""))

Not sure what @Hahnfeld thinks about this solution :)

The cleaner solution would probably be to define a %no-as-needed-flag substitution.

I'd prefer the second solution :D bonus points for a CMake check to see if the linker actually supports the flag. However, I'm not sure there is an OS except Darwin where the linker doesn't have that flag, so ...

simoatze updated this revision to Diff 153965.Jul 3 2018, 1:55 PM
simoatze edited the summary of this revision. (Show Details)

I updated the patch and the description.
I have no way at this moment to test it on Windows, is the "--no-as-needed" flag supported?

simoatze retitled this revision from Removed "--no-as-needed" flag from OMPT tests to avoid failures on OSX to Dropped "--no-as-needed" flag from OMPT for macOS.Jul 3 2018, 1:56 PM
simoatze retitled this revision from Dropped "--no-as-needed" flag from OMPT for macOS to Dropped non-supoorted "--no-as-needed" flag from OMPT tests for macOS.Jul 4 2018, 10:48 AM
This revision is now accepted and ready to land.Jul 5 2018, 2:11 AM

@simoatze you might want to add a bot building the OpenMP runtime upon each commit to make sure there are no regressions like this...

This revision was automatically updated to reflect the committed changes.