[PATCH] D142986: Enable multilib.yaml in the BareMetal ToolChain

Wed Feb 22 06:40:53 PST 2023

michaelplatings added inline comments.

================
Comment at: clang/test/Driver/baremetal-multilib.cpp:14
+// RUN:   | FileCheck %s
+// CHECK: "{{.*}}clang{{.*}}" "-cc1" "-triple" "thumbv8m.main-none-unknown-eabihf"
+// CHECK-SAME: "-internal-isystem" "{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include/c++/v1"
----------------
MaskRay wrote:
> With `-cc1` CHECK lines, remove `clang{{.*}}` before `-cc1`, then remove `-no-canonical-prefixes`. See my prior commits in this area.
I found your commit 980679981fbc311bc07f8cd23e3739fd56c22d2a which removes many uses of `-no-canonical-prefixes`. However in this case the test relies on `-no-canonical-prefixes` to ensure that clang finds the correct `lib/clang-runtimes` directory so it's necessary to keep it.

I have removed `clang{{.*}}` which was a copy-paste from `baremetal-sysroot.cpp` and applied similar tweaks like your tidy-up to ld.

================
Comment at: clang/test/Driver/baremetal-multilib.cpp:19
+// CHECK-NEXT: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic"
+// CHECK-SAME: "-L{{.*}}/baremetal_multilib/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib"
+// CHECK-SAME: "-lc" "-lm" "-lclang_rt.builtins"
----------------
MaskRay wrote:
> Use a style similar to `linux-cross.cpp`.
> 
> Test a few variants (https://github.com/MaskRay/Config/wiki/LLVM#clanglibdriver)
> Use a style similar to `linux-cross.cpp`.

I think you're referring to the use of `-L[[SYSROOT]]` instead of `-L{{.*}}` and I've made that change.

> Test a few variants (https://github.com/MaskRay/Config/wiki/LLVM#clanglibdriver)

The test follows the same pattern as `baremetal-sysroot.cpp` so in theory if that works on all the variants then so should this.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142986/new/

https://reviews.llvm.org/D142986