[PATCH] D126192: [Driver] Improve linking options for target AVR

haowei added a comment. I tested this patch on our windows builder and it passed as well. This patch LGTM. (I cannot click accept as the this review is already closed) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 __

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. In D126192#3583924 , @haowei wrote: > In D126192#3583915 , @benshi001 > wrote: > >> It seems the file >> `/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree/usr/bin

[PATCH] D126192: [Driver] Improve linking options for target AVR

haowei added a comment. In D126192#3583915 , @benshi001 wrote: > It seems the file > `/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld` > is missing on your machine, which I have newly created in current patch. That

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. It seems the file `/mnt/nvme_sec/SRC/llvm-project/clang/test/Driver/Inputs/basic_avr_tree/usr/bin/ld.lld` is missing on your machine, which I have newly created in current patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. In D126192#3583886 , @haowei wrote: > Emmm, I applied your latest patch on top of > main(6e02e27536b9de25a651cfc9c2966ce471169355 > ) but I > still get a te

[PATCH] D126192: [Driver] Improve linking options for target AVR

haowei added a comment. Emmm, I applied your latest patch on top of main(6e02e27536b9de25a651cfc9c2966ce471169355 ) but I still get a test failure on `Clang :: Driver/avr-toolchain.c`. `Clang :: Driver/avr-ld.c` no longer fai

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. In D126192#3583795 , @haowei wrote: > I already reverted the change in > https://github.com/llvm/llvm-project/commit/7fae15f9251d3b392058e2fc84898b53619d36ad > . It is likely the breakage is caused by the fact that Fuchsia's C

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 updated this revision to Diff 436992. benshi001 added a reviewer: haowei. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/AVR.cpp clang/test/Dr

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. I think I have fixed in another patch https://reviews.llvm.org/D127814, could you please have a look? @haowei I have not your environment, my patch works on my ubuntu system, could you please try it on your build machine ? Repository: rG LLVM Github Monorepo CHA

[PATCH] D126192: [Driver] Improve linking options for target AVR

haowei added a comment. I already reverted the change in https://github.com/llvm/llvm-project/commit/7fae15f9251d3b392058e2fc84898b53619d36ad . It is likely the breakage is caused by the fact that Fuchsia's Clang by default use `lld`, see `clang/cmake/caches/Fuchsia-stage2.cmake:set(CLANG_DEFA

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. I will fix it soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D126192: [Driver] Improve linking options for target AVR

haowei added a comment. We are seeing Clang driver test failures after this patch. On Fuchsia's mac clang builder, `Clang :: Driver/avr-ld.c` and `Clang :: Driver/avr-toolchain.c` are failing. Failed build task: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-mac-x64/b8811424

[PATCH] D126192: [Driver] Improve linking options for target AVR

This revision was automatically updated to reflect the committed changes. Closed by commit rGd7599be9e84f: [Driver] Improve linking options for target AVR (authored by benshi001). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://rev

[PATCH] D126192: [Driver] Improve linking options for target AVR

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. It is fine if you don't add a magic linker script for -fuse-ld=lld. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 _

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. a gentle ping ... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 marked an inline comment as done. benshi001 added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:507 +// Add user specified linker script. +const Arg *LDS = Args.getLastArg(options::OPT_T); +if (LDS) { MaskRay wrote: > Just

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 updated this revision to Diff 434684. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/AVR.cpp clang/test/Driver/avr-toolchain.c Index: clang/t

[PATCH] D126192: [Driver] Improve linking options for target AVR

MaskRay added inline comments. Comment at: clang/lib/Driver/ToolChains/AVR.cpp:507 +// Add user specified linker script. +const Arg *LDS = Args.getLastArg(options::OPT_T); +if (LDS) { Just do Args.AddAllArgs. Please check how Gnu.cpp passes `-T` CH

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. ping I have simplified my patch. Only support `-T` and `-fuse-ld=lld` according to user specification. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 ___ cfe-commit

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. I have a bit confused, why `clang --target=x86_64 -fuse-ld=lld` needs not to be explicitly specified with a linker script and produces a runnable x86_64 ELF? How does lld arrange code/data addresses for x86_64 by default ? My original will is `clang --target=avr -fuse

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 added a comment. In D126192#3549238 , @MaskRay wrote: > I am still uncomfortable with such a change. Trying to be smart sometimes may > get in the way. I have removed the guess of default linker script. Currently my patch only does 1. Support

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 updated this revision to Diff 433300. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/AVR.cpp clang/test/Driver/avr-toolchain.c Index: clang/t

[PATCH] D126192: [Driver] Improve linking options for target AVR

benshi001 updated this revision to Diff 433299. benshi001 retitled this revision from "[Driver] Support linking with lld for target AVR" to "[Driver] Improve linking options for target AVR". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126192/new/ https://reviews.llvm.org/D126192 Files