RE: [PATCH v2] RISC-V: Fix a typo in zce to zcf implication

[Yuriy Kolerov]

Hi Jeff,

That check is performed in a lambda function:

  {"zce", "zcf",
   [] (const riscv_subset_list *subset_list) -> bool
   {
     return subset_list->xlen () == 32 && subset_list->lookup ("f");
   }},

The typo was in a rule itself:

{"zcf", "f", ...

So, with this fix zcf in implied by zce if this condition is passed:

subset_list->xlen () == 32 && subset_list->lookup ("f")

Before 9e12010b5e724277ea this rule was implemented in this code:

  if (subset_list->lookup ("zce") != NULL
        && subset_list->m_xlen == 32
        && subset_list->lookup ("f") != NULL
        && subset_list->lookup ("zcf") == NULL)
    subset_list->add ("zcf", false);

But it was accidentally refactored in a wrong way.

Regards,
Yuriy Kolerov

-----Original Message-----
From: Jeff Law <jeffreya...@gmail.com> 
Sent: Tuesday, February 25, 2025 4:46 PM
To: Yuriy Kolerov <ykole...@synopsys.com>; gcc-patches@gcc.gnu.org
Cc: Artemiy Volkov <arte...@synopsys.com>
Subject: Re: [PATCH v2] RISC-V: Fix a typo in zce to zcf implication



On 2/24/25 3:22 AM, Yuriy Kolerov wrote:
> zce must imply zcf but this rule was corrupted after refactoring in 
> 9e12010b5e724277ea. This may be observed ater generating an .s file 
> from any source code file with -mriscv-attribute -march=rv32if_zce 
> -mabi=ilp32 -S options. A full march will be presented in arch 
> attribute:
> 
>      rv32i2p1_f2p2_zicsr2p0_zca1p0_zcb1p0_zce1p0_zcmp1p0_zcmt1p0
> 
> As you see, zcf is not presented here though f_zce pair is passed in 
> -march. According to The RISC-V Instruction Set Manual:
> 
>      Specifying Zce on RV32 with F includes Zca, Zcb, Zcmp,
>      Zcmt and Zcf.
> 
>       PR target/118906
> 
> gcc/ChangeLog:
> 
>       * common/config/riscv/riscv-common.cc: fix zce to zcf
>       implication.
> 
> gcc/testsuite/ChangeLog:
> 
>       * gcc.target/riscv/attribute-zce-1.c: New test.
>       * gcc.target/riscv/attribute-zce-2.c: New test.
>       * gcc.target/riscv/attribute-zce-3.c: New test.
>       * gcc.target/riscv/attribute-zce-4.c: New test.
I'm not 100% sure this is implementation is correct.  My understanding is that 
zce implies zcf iff rv32 and f are also enabled.  So don't you need to verify 
that rv32 and f are enabled?  Or is that done elsewhere?

Though it looks like the other cases zce -> {zca, zcb, zcmp, zcmp} don't have 
that check.

It feels like I'm missing something in how all this works.

Kito, you know this stuff better than I, thoughts?


Jeff