[PATCH] D140959: RFC: Multilib prototype

2023-02-01 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.

FWIW I tried this patch out with RISC-V multilibs, and it works a treat. It 
solves the multilib reuse problem and relaxes the order of non-standard 
extensions. I did have to copy some of the logic from BareMetal.cpp to Gnu.cpp 
as was using a GCC installation.

The `args` field for the YAML was unnecessary and worked (including 
--print-multi-lib) just fine without it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140959

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142933: Add -print-multi-selection-flags argument

2023-02-01 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.

So again, I took these patches for a spin with RISC-V. As I mentioned before, I 
was using a GCC installation, so the multilib selection happens inside Gnu.cpp 
(findRISCVBareMetalMultilibs). The main trouble I had was at that point we 
don't have a reference to the ToolChain, so calling `getMultiSelectionFlags` 
was not possible. I had a go at making this function static, and it was mostly 
fine as you could just pass in the Triple and Driver, however 
`getSanitizerArgs` was not possible. I'm not sure what the solution here is.

Multilibs for RISCV are straightforward, they just depend arch+abi, so I added 
the arch extensions and abi to the Result in `getMultiSelectionFlags`.

With that modified `getMultiSelectionFlags`, it worked well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142933: Add -print-multi-selection-flags argument

2023-02-02 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.



>> The main trouble I had was at that point we don't have a reference to the 
>> ToolChain, so calling `getMultiSelectionFlags` was not possible. [...] I'm 
>> not sure what the solution here is.
>
> I see two options:
>
> 1. Split getMultiSelectionFlags into needs-ToolChain and 
> doesn't-need-ToolChain parts, with the former calling the latter. From 
> Gnu.cpp, call the latter, with the limitation that it can use fewer flags.
> 2. Pass a ToolChain to the Gnu.cpp functions.

I did briefly try to pass a ToolChain to Gnu.cpp functions, but it became 
intrusive and had to add it in 5+ places, so unless I'm missing a trick to get 
the ToolChain from the Driver, my vote would go to the former.

>> I added the arch extensions and abi to the Result in `getMultiSelectionFlags`
>
> What was the syntax you used for that?
> I'm not super keen on the `march=+ext` syntax I came up with so I'm open to 
> alternatives.

I dropped the `march=` and used the same format as target-features, so it was 
simply `+m` or `+a`. I think this is intuitive enough without the `march=`, and 
the form `x=y` is already broken when you add the flags from the flag list. I 
don't have a strong stance on this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142933: Add -print-multi-selection-flags argument

2023-02-02 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.



> Thanks for explaining. I found that when I was writing a regex to match flags 
> it was helpful to have a part of the string I could be sure would be there to 
> avoid matching the wrong type of flag, and `march=` helped with that. I also 
> worry that names from different types of flags could clash without some kind 
> of namespacing.

Understandable about the flag names clashing. The scope of riscv multilibs was 
just arch/abi, so there was no concern about clashing/matching the wrong type 
of flag. Would it be weird for one target to have the `march=` but anothers not?




Comment at: clang/lib/Driver/Driver.cpp:2213
+  if (C.getArgs().hasArg(options::OPT_print_multi_selection_flags)) {
+for (StringRef Attr : TC.getMultiSelectionFlags(C.getArgs()))
+  llvm::outs() << Attr << '\n';

Do we want to parse the multilib.yaml here so we can print out custom flags as 
well? It could help diagnose issues people have with them.



Comment at: clang/lib/Driver/ToolChain.cpp:204
+Result.push_back(
+clang::driver::getDriverOptTable().getOptionName(Option).str());
+  }

> 
>> the form `x=y` is already broken when you add the flags from the flag list.
> 
> Can you give an example of what you mean by that? Sounds like something that 
> might need fixing.
For example the option name for OPT_fexceptions is just `fexceptions`, and this 
is added directly to `Results`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D155294: [Driver][RISCV] Find baremetal multilibs using YAML for GNU toolchain

2023-07-24 Thread Joseph Faulls via Phabricator via cfe-commits
Joe added a comment.

> Theoretically, I think this matching of multilibs could be done without the 
> need for multilib.yaml, but it is indeed much easier considering the logic is 
> already there.

Actually, I retract this statement. There would be no way of knowing what the 
default library is without something telling you, i.e. a yaml file.

@michaelplatings thank you for the speedy comment!

> Multilib flags must be valid command line options. So as `+a` is not a valid 
> command line option, it should not be used as a multilib flag.
>
> That said, I'm very aware that it would be desirable to specify attributes 
> independently of command line options, so I think there's room for the design 
> to grow.

Matching on extension is the most important thing for riscv. The only other way 
to do it under the current vision of multilib flags, as far as I'm aware, are 
some nasty and repetitive regexes. Do you think we could allow the design to 
grow to allow attributes prefixed by `+` in addition to flags?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155294

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D155294: [Driver][RISCV] Find baremetal multilibs using YAML for GNU toolchain

2023-07-24 Thread Joseph Faulls via Phabricator via cfe-commits
Joe updated this revision to Diff 543440.
Joe added a comment.

Add default ABI to multilib flags if none supplied


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

https://reviews.llvm.org/D155294

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/bin/ld
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/multilib.yaml
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32i/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32im/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32imac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o
  clang/test/Driver/riscv-toolchain-multilib-yaml.c

Index: clang/test/Driver/riscv-toolchain-multilib-yaml.c
===
--- /dev/null
+++ clang/test/Driver/riscv-toolchain-multilib-yaml.c
@@ -0,0 +1,79 @@
+// UNSUPPORTED: system-windows
+// In this test, multilib resolution is resolved using a multilib.yaml
+// definition, in which multilibs are picked in accordance to matching abi /
+// marchs.
+
+// RUN: env "PATH=" %clang -### %s -fuse-ld=ld \
+// RUN:   --target=riscv32-unknown-elf --rtlib=platform --sysroot= \
+// RUN:   -march=rv32iac -mabi=ilp32\
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk_yaml 2>&1 \
+// RUN:   | FileCheck -check-prefix=C-RV32IAC-BAREMETAL-MULTI-ILP32 %s
+
+// C-RV32IAC-BAREMETAL-MULTI-ILP32: "{{.*}}/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib/rv32iac/ilp32{{/|}}crt0.o"
+// C-RV32IAC-BAREMETAL-MULTI-ILP32: "{{.*}}/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32{{/|}}crtbegin.o"
+// C-RV32IAC-BAREMETAL-MULTI-ILP32: "{{.*}}/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32{{/|}}crtend.o"
+
+// RUN: env "PATH=" %clang -### %s -fuse-ld=ld \
+// RUN:   --target=riscv32-unknown-elf --rtlib=platform --sysroot= \
+// RUN:   -march=rv32imafc -mabi=ilp32f\
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk_yaml 2>&1 \
+// RUN:   | FileCheck -check-prefix=C-RV32IMAFC-BAREMETAL-MULTI-ILP32F %s
+
+// C-RV32IMAFC-BAREMETAL-MULTI-ILP32F: "{{.*}}/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib/rv32imafc/ilp32f{{/|}}crt0.o"
+// C-RV32IMAFC-BAREMETAL-MULTI-ILP32F: "{{.*}}/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp

[PATCH] D155294: [Driver][RISCV] Find baremetal multilibs using YAML for GNU toolchain

2023-07-14 Thread Joseph Faulls via Phabricator via cfe-commits
Joe created this revision.
Joe added reviewers: michaelplatings, kito-cheng.
Joe added a project: clang.
Herald added subscribers: jobnoorman, luke, shiva0217, VincentWu, vkmr, 
frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, abidh, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, 
asb, kristof.beyls, arichardson, emaste.
Herald added a project: All.
Joe requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, eopXD, MaskRay.

This patch allows usage of multilib.yaml 
 when using a riscv gnu 
toolchain

This could certainly land as three separate patches, but I wanted to put this 
up as one to gather feedback to make sure the direction makes sense.

These are the main discussion points I want to bring to attention:

1. For the RISCVMultilibFlags, the mabi flag is used in combination with all 
the march extension features (e.g +m). Notably, this doesn't align with the 
current arm/aarch64 multilib flags, which all flags corresponding to the 
command line flag. E.G `-march=`. Does this violate a particular design 
decision, or can any target decide on whatever multilib flags they want?

2. The location of multilib.yaml for a gnu toolchain is in the lib directory of 
the sysroot. E.G `riscv64-unknown-elf/lib/multilib.yaml`. This differs from the 
baremetal location of "lib/clang-runtimes".

3. Does it make more sense to implement finding multilibs using yaml for riscv 
in the baremetal toolchain first? I was planning on doing it after.

Theoretically, I think this matching of multilibs could be done without the 
need for multilib.yaml, but it is indeed much easier considering the logic is 
already there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155294

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32i/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32iac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32im/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imac/ilp32/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv32imafc/ilp32f/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imac/lp64/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/lib/gcc/riscv64-unknown-elf/8.2.0/rv64imafdc/lp64d/crtend.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/bin/ld
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/multilib.yaml
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32i/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32iac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32im/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32imac/ilp32/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv32imafc/ilp32f/crt0.o
  
clang/test/Driver/Inputs/multilib_riscv_elf_sdk_yaml/riscv64-unknown-elf/lib/rv64imac/lp64/crt0.o
  clang/test/Driver/riscv-toolchain-multilib-yaml.c

Index: clang/test/Driver/riscv-toolchain-multilib-yaml.c
===
--- /dev/null
+++ clang/test/Driver/riscv-toolch