On Fri, Dec 15, 2023 at 12:36 AM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
>
> On 12/14/23 02:46, Christoph Müllner wrote:
> > On Tue, Jun 20, 2023 at 12:34 AM Jeff Law via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >> A handful of the scalar crypto instructions are supposed to take a
> >> constant integer argument 0..3 inclusive.  A suitable constraint was
> >> created and used for this purpose (D03), but the operand's predicate is
> >> "register_operand".  That's just wrong.
> >>
> >> This patch adds a new predicate "const_0_3_operand" and fixes the
> >> relevant insns to use it.  One could argue the constraint is redundant
> >> now (and you'd be correct).  I wouldn't lose sleep if someone wanted
> >> that removed, in which case I'll spin up a V2.
> >>
> >> The testsuite was broken in a way that made it consistent with the
> >> compiler, so the tests passed, when they really should have been issuing
> >> errors all along.
> >>
> >> This patch adjusts the existing tests so that they all expect a
> >> diagnostic on the invalid operand usage (including out of range
> >> constants).  It adds new tests with proper constants, testing the
> >> extremes of valid values.
> >>
> >> OK for the trunk, or should we remove the D03 constraint?
> >
> > Reviewed-by: Christoph Muellner <christoph.muell...@vrull.eu>
> >
> > The patch does not apply cleanly anymore, because there were some
> > small changes in crypto.md.
> Here's an update to that old patch that also takes care of the pattern
> where we allow 0..10 inclusive, but not registers.
>
> Regression tested on rv64gc without new failures.  It'll need a
> ChangeLog when approved, but that's easy to adjust.

Looks good and tests pass for rv64gc and rv32gc.

Reviewed-by: Christoph Muellner <christoph.muell...@vrull.eu>
Tested-by: Christoph Muellner <christoph.muell...@vrull.eu>

Reply via email to