DavidSpickett wrote: Put `-mcpu` aside for now, that's nice, but it's only going to be viable in a world where the backend (tablegen) and frontend (target parsers/ABI info) agree on what is valid and can also be passed to clang.
> The file with the full log outputs is here: > [sample-outputs.log.gz](https://github.com/llvm/llvm-project/files/12646975/sample-outputs.log.gz) This is problem 1 right now. RISC-V is currently printing more kinds of information than what your sample outputs are with this patch. Currently it prints: ``` $ ./bin/clang --target=riscv64-unknown-linux-gnu --print-supported-extensions <...> All available -march extensions for RISC-V Name Version i 2.1 e 2.0 <...> xperimental extensions zicfilp 0.2 ``` So you will have to either get the RISC-V folks to agree that that extra information is not required, or pipe that info through `getAllProcessorFeatures` and have the higher level code know to split extensions into sections. I believe RISC-V is the only backend to have versions and status (at least explicitly), but given it motivated the feature in the first place, it should be catered for. If we can't handle what RISC-V needs, we've done something wrong. Problem 2 is that this patch is listing backend flags that are never meant to be passed in `march` or `mcpu` to clang. For example it's listing `call-saved-x10`. ``` $ ./bin/clang --target=aarch64-unknown-linux-gnu -march=armv8-a+call-saved-x10 /tmp/test.c clang: error: unsupported argument 'armv8-a+call-saved-x10' to option '-march=' ``` It also includes feature names like `apple-a10` which if anything should go in `mcpu`, they clearly shouldn't be listed in an option that is showing valid options for `-march`. For AArch64, look at https://github.com/llvm/llvm-project/blob/102838d3f6e15a5c510257c2c70fe7faca5b59d6/llvm/include/llvm/TargetParser/AArch64TargetParser.h#L190. You cannot pass `+bti` to `-march=` (in this case because we want you to use other compiler options to generate these instructions, but that's beside the point). So what your patch produces is kinda like `./bin/clang --target <whatever> -mllvm -mattr=+unknown_name` and getting `Don't understand unknown_name, you could have passed <list of every possible backend feature>. (and we have had customers ask for exactly that sort of option, but there's a reason some can't be passed to clang) Unfortunately the information of what is valid to be passed to clang is not in the same tablegen you're pulling from. It would be nice, and it's not as far off a goal as it used to be, but we aren't there yet. I applaud the instinct to generalise and use a single source of information, but one has to consider the current state of affairs and what might be lost by moving forward with this. https://github.com/llvm/llvm-project/pull/66586 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
