This is an archive of the discontinued LLVM Phabricator instance.

[Hexagon] Select lld as the default linker for linux-musl target
ClosedPublic

Authored by sidneym on Apr 5 2020, 7:14 AM.

Details

Summary

When the target is hexagon-unknown-linux-musl select lld as the default linker.

Diff Detail

Event Timeline

sidneym created this revision.Apr 5 2020, 7:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2020, 7:14 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
kparzysz accepted this revision.Apr 6 2020, 5:52 AM
This revision is now accepted and ready to land.Apr 6 2020, 5:52 AM
This revision was automatically updated to reflect the committed changes.

This does not take into account CLANG_DEFAULT_LINKER, resulting in a check-clang failure:

$ cmake -GNinja \
              -DCMAKE_C_COMPILER=clang \
              -DCMAKE_CXX_COMPILER=clang++ \
              -DLLVM_ENABLE_PROJECTS=clang \
              -DPYTHON_EXECUTABLE=$(command -v python3) \
              ../llvm && ninja check-clang
...
Testing Time: 163.80s
  Expected Passes    : 17055
  Expected Failures  : 23
  Unsupported Tests  : 55

$ cmake -GNinja \
              -DCMAKE_C_COMPILER=clang \
              -DCMAKE_CXX_COMPILER=clang++ \
              -DLLVM_ENABLE_PROJECTS=clang \
              -DPYTHON_EXECUTABLE=$(command -v python3) \
              -DCLANG_DEFAULT_LINKER=lld \
              ../llvm && ninja check-clang
...
Testing Time: 176.55s
********************
Failing Tests (1):
    Clang :: Driver/hexagon-toolchain-elf.c

  Expected Passes    : 17050
  Expected Failures  : 23
  Unsupported Tests  : 59
  Unexpected Failures: 1
bcain added a comment.EditedApr 7 2020, 2:29 PM

This does not take into account CLANG_DEFAULT_LINKER, resulting in a check-clang failure:

Weird -- the baseline didn't take CLANG_DEFAULT_LINKER into account either, though, right? Is it a regression?

Weird -- the baseline didn't take CLANG_DEFAULT_LINKER into account either, though, right?

I assume by baseline you mean the test before this change? It does not seem like it.

Is it a regression?

I guess not but the test is still broken when CLANG_DEFAULT_LINKER is set.

These are new tests how do you get the generic lld driver to work? When I invoke it using just "lld" I need to add -flavor gnu or I get this error:
Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld (WebAssembly) instead

Since this isn't something that can be always known in advance I think the testcase should just be removed.

bcain added a comment.Apr 7 2020, 3:31 PM

Since this isn't something that can be always known in advance I think the testcase should just be removed.

Why not just check if CLANG_DEFAULT_LINKER is empty?

const char *getDefaultLinker() const override {
    StringRef ClangDefault = CLANG_DEFAULT_LINKER;
    return ClangDefault.isEmpty() ? (getTriple().isMusl() ? "ld.lld" : "hexagon-link") : ClangDefault.str();
  }

These are new tests how do you get the generic lld driver to work?

It doesn't, Clang invokes the appropriate driver in ToolChain::GetLinkerPath(): https://github.com/llvm/llvm-project/blob/6011627f5118dd64a0c33694b604c70e766d8c40/clang/lib/Driver/ToolChain.cpp#L537-L541