Hi Yuriy:
V2 is LGTM, thanks :)
On Wed, Feb 26, 2025 at 3:06 AM Yuriy Kolerov
<yuriy.kole...@synopsys.com> wrote:
>
> 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
