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

[Jeff Law <jeffreyalaw@gmail.com>]

On 11/17/22 16:56, Palmer Dabbelt wrote:
> 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.

I'm pretty confident Ventana won't be making a part without Zbs which is 
why I raised the issue


I also understand Philipp's position that one could explicitly turn on 
ventanacondops and zbs off and that there's a notable possibility that 
this ultimately turns into ZICondOps independent of Ventana.


So I guess we keep it...  But it also feels like a ticking time bomb WRT 
the ability to mix and match things the way we currently allow.  I 
suspect if we were to look at the full test matrix and deeply test that 
full matrix that we'd find a number of problems where two options 
interact badly.

Jeff