Committed, thanks, other improvement changes will be sent in separate patches in the next few days :)
On Thu, Oct 15, 2020 at 5:41 AM Jim Wilson <j...@sifive.com> wrote: > > On Tue, Oct 13, 2020 at 3:09 AM Kito Cheng <kito.ch...@sifive.com> wrote: > > - The behavior of -mcpu basically equal to -march plus -mtune, but it > > has lower priority than -march and -mtune. > > This looks OK to me. > > I noticed a few things while testing. These don't need to be fixed > before the patch is committed. > > Using an invalid cpu name results in two errors. > rohan:2116$ ./xgcc -B./ -mcpu=sifive-e51 tmp.c > cc1: error: ‘-mcpu=sifive-e51’: unknown CPU > cc1: error: unknown cpu ‘sifive-e51’ for ‘-mtune’ > Ideally that should be one error. The second error may be confusing > as the user did not specify the -mtune option. Maybe riscv_parse_tune > can have another option to indicate whether it was passed a tune > string or cpu string, and only error if it was given a tune string, > since the cpu string would already have given an error. Or maybe pass > in both the cpu and tune strings, and it can ignore the cpu string if > a tune string was specified. And only give an error if we are using > the tune string. You can also avoid passing the tune string to > riscv_find_cpu which doesn't appear to be useful. > > Using a valid cpu name that has different default arch than configured > gives a possibly confusing error > rohan:2117$ ./xgcc -B./ -mcpu=sifive-s51 tmp.c > cc1: error: requested ABI requires ‘-march’ to subsume the ‘D’ extension > This is for a toolchain with lp64d as default ABI. There is a > deliberate choice here not to force the ABI from the -mcpu option, but > maybe the error can be improved, as using -mabi to fix this is more > likely to be correct than using -march. Also, I never liked the use > of "subsume" here. We shouldn't force users to crack open a > dictionary to figure out what an error message means. Also, the user > didn't request an ABI, the compiler is using the configured default, > nor did the user specify a -march option. Maybe something like > cc1: error: ABI %s requires the 'D' extension, arch %s does not include it > where the %s expands to the ABi and arch that the compiler is using. > There is also another similar error that uses "subsume" to keep these > warnings consistent. If we want to fix this, this should be a > separate patch. > > Jim
