Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[Driver][RISCV] Find baremetal multilibs using YAML for GNU toolchain
Needs ReviewPublic

Authored by Joe on Jul 14 2023, 7:36 AM.

Details

Summary

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?
  1. 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".
  1. 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.

Diff Detail

Event Timeline

Joe created this revision.Jul 14 2023, 7:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 14 2023, 7:36 AM
Joe requested review of this revision.Jul 14 2023, 7:36 AM

Hi Joe, it's nice to see multilib.yaml getting some adoption :)

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?

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.

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".

The multilib.yaml design is agnostic to such decisions. If you can guarantee that all libraries will always use the exact same header files then it could make sense to have multilib apply only to libs. For the BareMetal toolchain I wanted to keep the flexibility of allowing different headers to be associated with each library.

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.

No strong opinion from me on this one. Doing BareMetal first might lend itself to a more consistent end result.

Joe added a comment.Jul 24 2023, 2:12 AM

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?

Joe updated this revision to Diff 543440.Jul 24 2023, 2:51 AM

Add default ABI to multilib flags if none supplied

some nasty and repetitive regexes

Yes, it's the same for Arm architecture extensions. Petr & I went back and forth on this for a long time. See discussion here to understand Petr's concerns: https://discourse.llvm.org/t/rfc-multilib/67494/23