quic-k wrote: thanks for review @smithp35
> Not knowing the hexagon toolchain, I can't give you any useful comments on > that part of the review. Based on our experience of Picolibc in the baremetal > driver the question I have for you is whether there is enough picolibc > specific details here to justify a specific picolibc environment. > picolibc enviroment is right now used to set -I and -L paths, and we add crt0-semihost and -lsemihost that we get from Picolibc, I don't know if this is enough to justify a new environment. But what else is Musl environment used for apart from what I listed? > It looks like the majority of the extra configuration that you are adding is > not specific to picolibc, but could also apply to other bare-metal > C-libraries like llvm libc and newlib. About the only thing that I can see > that is Picolibc specific is adding crt-semihost.o for the startfiles. > > Would it be better to add this as a bare-metal configuration, not specific to > picolibc, then you wouldn't need a new environment when changing C-library? > sry, I don't get what you mean my "add this as bare-metal configuration", the idea for adding this triple was so that C libraries can co-exist and can be switch by just switching the triple from say : hexagon-none-picolibc to hexagon-none-llvmlibc we would point -I and -L flags to the different dirs based on the environment, and add crt and other files > FWIW. We require our users to use `-nostartfiles -lcrt-semihost` if they > prefer the semihosting startup code (with semihosted argc, argv and return) > and `lsemihost` for the semihost implementation. > I did check https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm/blob/main/docs/newlib.md, I like the idea of pointing to correct sysroot using .cfg file, but isn't adding crt/semihost libs too much work for users? wouldn't it be better if the user can just specify triple or/and --libc flag and clang driver takes care of everything else? > It may also be useful to respond to > https://discourse.llvm.org/t/rfc-add-command-line-option-for-selecting-c-library/87335 > as the environment part is acting that way. > --libc flag would have been perfect for our usecase, but I don't know if this is going to go in. if we could simply use `libc.isPicolibc()` instead of `triple.isPicolibc()`, that would have been great > While I don't have any strong objections to adding picolibc as an evironment, > there was some pushback to the RFC as there were so many bare-metal > C-libraries, do we end up with an environment for each of them? With that in > mind it may be better to split the generic triple parts out into a separate > patch, to get more opinions on whether it is the right thing to do. I have split it into 2 commits, do you prefer it to be a separate PR instead? https://github.com/llvm/llvm-project/pull/169613 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
