Re: [PATCH 4/7] RISC-V: Recognize sign-extract + and cases for XVentanaCondOps

[Palmer Dabbelt <palmer@rivosinc.com>]

On Thu, 17 Nov 2022 15:41:26 PST (-0800), gcc-patches@gcc.gnu.org wrote:
>
> On 11/12/22 14:29, Philipp Tomsich wrote:
>> Users might use explicit arithmetic operations to create a mask and
>> then and it, in a sequence like
>>      cond = (bits >> SHIFT) & 1;
>>      mask = ~(cond - 1);
>>      val &= mask;
>> which will present as a single-bit sign-extract.
>>
>> Dependening on what combination of XVentanaCondOps and Zbs are
>> available, this will map to the following sequences:
>>   - bexti + vt.maskc, if both Zbs and XVentanaCondOps are present
>>   - andi + vt.maskc, if only XVentanaCondOps is available and the
>>                      sign-extract is operating on bits 10:0 (bit
>> 		    11 can't be reached, as the immediate is
>> 		    sign-extended)
>>   - slli + srli + and, otherwise.
>>
>> gcc/ChangeLog:
>>
>> 	* config/riscv/xventanacondops.md: Recognize SIGN_EXTRACT
>> 	  of a single-bit followed by AND for XVentanaCondOps.
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>> ---
>>
>>   gcc/config/riscv/xventanacondops.md | 46 +++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/gcc/config/riscv/xventanacondops.md b/gcc/config/riscv/xventanacondops.md
>> index 7930ef1d837..3e9d5833a4b 100644
>> --- a/gcc/config/riscv/xventanacondops.md
>> +++ b/gcc/config/riscv/xventanacondops.md
>> @@ -73,3 +73,49 @@
>>     "TARGET_XVENTANACONDOPS"
>>     [(set (match_dup 5) (match_dup 1))
>>      (set (match_dup 0) (and:X (neg:X (ne:X (match_dup 5) (const_int 0)))
>> +
>> +;; Users might use explicit arithmetic operations to create a mask and
>> +;; then and it, in a sequence like
>
> Nit.  Seems like a word is missing.  "make and then and it"??
>
>
> Do we really care about TARGET_XVENTANACONDOPS && ! TARGET_ZBS?

I guess that's really more of a question for the Ventana folks, but 
assuming all the Ventana widgets have Zbs then it seems reasonable to 
just couple them -- there's already enough options in RISC-V land to 
test everything, might as well make sure what slips through the cracks 
isn't being built.

Probably best to have a comment saying why here, and then something to 
enforce the dependency in -march (either as an implict extension 
dependency, or just a warning/error) so users don't get tripped up on 
configs that aren't expected to work.

> If there's a good reason to care about the !TARGET_ZBS case, then OK
> with the nit fixed.   If we agree that the !TARGET_ZBS case isn't all
> that important, then obviously OK with that pattern removed too.
>
> I'm about out of oomph today.  I may take a look at 7/7 tonight though. 
> Given it hits target independent code we probably want to get resolution
> on that patch sooner rather than later.

Thanks, there's no way we would have gotten this all sorted out so fast 
without the help!