This is an archive of the discontinued LLVM Phabricator instance.

[hexagon] restore library path arguments
ClosedPublic

Authored by bcain on Jul 26 2023, 6:42 AM.

Details

Summary

Before applying this fix, clang would not include the specified library
path arguments:

$ ./bin/clang --target=hexagon-unknown-linux-musl  -o tprog tprog.o -L/tmp -###
...
clang: warning: argument unused during compilation: '-L/tmp' [-Wunused-command-line-argument]
 "/local/mnt/workspace/install/clang-latest/bin/ld.lld" "-z" "relro" "-o" "tprog" "-dynamic-linker=/lib/ld-musl-hexagon.so.1" "/usr/lib/crt1.o" "-L/usr/lib" "tprog.o" "-lclang_rt.builtins-hexagon" "-lc"

Diff Detail

Event Timeline

bcain created this revision.Jul 26 2023, 6:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 6:42 AM
bcain requested review of this revision.Jul 26 2023, 6:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 6:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MaskRay accepted this revision.Jul 26 2023, 10:29 AM

Tip: use %clang -fdriver-only -Werror -v to test that a command produces no warning or error and has an exit code of 0. Without it your Args.ClaimAllArgs(options::OPT_L); change is untested.

clang/test/Driver/hexagon-toolchain-linux.c
123

-target has been deprecated since Clang 3.4. Prefer --target= for new tests.
I think you can place %s on this %clang ... line.

125

too early wrapping?

129

Prefer quoting driver's cc1 output, "-L/tmp"

This revision is now accepted and ready to land.Jul 26 2023, 10:29 AM
bcain added a comment.Jul 26 2023, 2:20 PM

Tip: use %clang -fdriver-only -Werror -v to test that a command produces no warning or error and has an exit code of 0. Without it your Args.ClaimAllArgs(options::OPT_L); change is untested.

This test fails on the baseline because of the CHECK-NOT: warning: IIRC. But your suggestion sounds like a good one anyways, will do!

bcain updated this revision to Diff 544527.Jul 26 2023, 2:52 PM

Fixed target: --target=hexagon-unknown-linux was not correct for testing this bug, it should have been --target=hexagon-unknown-linux-musl.

Used -fdriver-only -Werror as suggested but it did require separating the tests.

Quoted the library path arg, to make match from -cc1 command stricter, from review suggestion.

Fixed wrapping.

Fixed target: --target=hexagon-unknown-linux was not correct for testing this bug, it should have been --target=hexagon-unknown-linux-musl.

Used -fdriver-only -Werror as suggested but it did require separating the tests.

Quoted the library path arg, to make match from -cc1 command stricter, from review suggestion.

Fixed wrapping.

-fdriver-only -v -Werror is preferred because -### has a quirk. I hope we will fix it in the near future: D156363 :) Your change doesn't need to be blocked by that.

bcain updated this revision to Diff 544532.Jul 26 2023, 2:59 PM

I misunderstood the previous suggestion about -fdriver-only -Werror -v and thought I needed to separate the test into two invocations. Combined these back into a single RUN now.

bcain updated this revision to Diff 544533.Jul 26 2023, 3:01 PM

Too hasty w/the last update - forgot to remove the -###.

This revision was landed with ongoing or failed builds.Jul 27 2023, 1:26 PM
This revision was automatically updated to reflect the committed changes.