Re: [PATCH 1/1] RISC-V: Add support for XCVbitmanip extension in CV32E40P

[Jeff Law <jeffreyalaw@gmail.com>]

On 12/5/23 08:30, Kito Cheng wrote:
>> index 7d7b952d817..e7d4ad1760c 100644
>> --- a/gcc/config/riscv/corev.md
>> +++ b/gcc/config/riscv/corev.md
>> @@ -27,6 +27,25 @@
>>
>>     ;;CORE-V EVENT LOAD
>>     UNSPECV_CV_ELW
>> +
>> +  ;;CORE-V BITMANIP
>> +  UNSPEC_CV_BITMANIP_EXTRACT
>> +  UNSPEC_CV_BITMANIP_EXTRACT_INSN
>> +  UNSPEC_CV_BITMANIP_EXTRACTR_INSN
>> +  UNSPEC_CV_BITMANIP_EXTRACTU
>> +  UNSPEC_CV_BITMANIP_EXTRACTU_INSN
>> +  UNSPEC_CV_BITMANIP_EXTRACTUR_INSN
>> +  UNSPEC_CV_BITMANIP_INSERT
>> +  UNSPEC_CV_BITMANIP_INSERT_INSN
>> +  UNSPEC_CV_BITMANIP_INSERTR_INSN
> 
> You could reference bfe, sbfx and ubfx instructions in aarch64.md
> to see how to write the insert and extract bit with RTL code.
> 
>> +  UNSPEC_CV_BITMANIP_BCLR
>> +  UNSPEC_CV_BITMANIP_BCLR_INSN
>> +  UNSPEC_CV_BITMANIP_BCLRR_INSN
>> +  UNSPEC_CV_BITMANIP_BSET
>> +  UNSPEC_CV_BITMANIP_BSET_INSN
>> +  UNSPEC_CV_BITMANIP_BSETR_INSN
> 
> Just use generic RTL code for bset and bclr is fine, you could
> reference bitmanip.md
Agreed.  And as a general principle if we can reasonably express the 
semantics of an instruction with RTL, we generally should.  Doing so 
gives the optimizers a chance to improve stuff.

I haven't looked at the patches, but the same might apply to the 
extractions & insertions, though there's more complex in that there's 
multiple implementations and I suspect some general cleanups would 
likely be necessary for that to work.  We started to look at it a bit, 
but concluded there were bigger fish to fry.

jeff