Re: [PATCH 2/7] riscv: riscv-cores.def: Add T-Head XuanTie C906

[Palmer Dabbelt <palmer@dabbelt.com>]

On Thu, 17 Nov 2022 20:50:19 PST (-0800), gcc-patches@gcc.gnu.org wrote:
> On Sun, Nov 13, 2022 at 10:46:31PM +0100, Christoph Muellner wrote:
>> From: Christoph Müllner <christoph.muellner@vrull.eu>
>>
>> This adds T-Head's XuanTie C906 to the list of known cores as "thead-c906".
>> The C906 is shipped for quite some time (it is the core of the Allwinner D1).
>> Note, that the tuning struct for the C906 is already part of GCC (it is
>> also name "thead-c906").
>>
>> gcc/ChangeLog:
>>
>> 	* config/riscv/riscv-cores.def (RISCV_CORE): Add "thead-c906".
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* gcc.target/riscv/mcpu-thead-c906.c: New test.
>>
>> Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu>
>> ---
>>  gcc/config/riscv/riscv-cores.def               |  2 ++
>>  .../gcc.target/riscv/mcpu-thead-c906.c         | 18 ++++++++++++++++++
>>  2 files changed, 20 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.target/riscv/mcpu-thead-c906.c
>>
>> diff --git a/gcc/config/riscv/riscv-cores.def b/gcc/config/riscv/riscv-cores.def
>> index 31ad34682c5..648a010e09b 100644
>> --- a/gcc/config/riscv/riscv-cores.def
>> +++ b/gcc/config/riscv/riscv-cores.def
>> @@ -73,4 +73,6 @@ RISCV_CORE("sifive-s76",      "rv64imafdc", "sifive-7-series")
>>  RISCV_CORE("sifive-u54",      "rv64imafdc", "sifive-5-series")
>>  RISCV_CORE("sifive-u74",      "rv64imafdc", "sifive-7-series")
>>
>> +RISCV_CORE("thead-c906",      "rv64imafdc", "thead-c906")
>> +
>
> I think it makes more sense that thead-906 includes extended instructions by default.

Seems reasonable to me, but Kito understands this stuff better than I 
do.  IMO `-mtune=thead-c906` should leave the ISA targets alone and just 
set the tune info, and `-mcpu=thead-c906` should do that and also set 
the ISA to whatever's implemented on that core.

That said, I was playing around with some B-extension multilib stuff 
recently and am pretty sure this stuff is all a bit broken.  Maybe we 
should punt on enabling all these extensions for `-mcpu` until we have 
that sorted out?  IMO we're at the point where having ISA-dependent 
multilib paths on Linux makes sense, but that risks throwing another 
wrench into distro folks.

Maybe it doesn't matter, though?  IIUC distros aren't shipping multilib 
right now so the bugs won't manifest for users.