Re: [PATCH] i386: Fix up *add_1 [PR125469]

[Uros Bizjak]

On Thu, May 28, 2026 at 1:16 PM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Thu, May 28, 2026 at 01:02:46PM +0200, Uros Bizjak wrote:
> > Like the attached patch?
>
> Note, I've already committed my patch to trunk, so this won't apply there.
>
> > I guess we should change all similar TYPE_LEA alternatives (ADD and SAL 
> > insns).
> >
> > Uros.
>
> > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
> > index 063e14cb2c1..baa3a6f8db7 100644
> > --- a/gcc/config/i386/i386.md
> > +++ b/gcc/config/i386/i386.md
> > @@ -983,6 +983,8 @@ (define_attr "enabled" ""
> >          (symbol_ref "TARGET_APX_NDD")
> >        (eq_attr "isa" "apx_ndd_64")
> >          (symbol_ref "TARGET_APX_NDD && Pmode == DImode")
> > +      (eq_attr "isa" "noapx_nf")
> > +        (symbol_ref "!TARGET_APX_NF")
> >        (eq_attr "isa" "vaes_avx512vl")
> >          (symbol_ref "TARGET_VAES && TARGET_AVX512VL")
> >        (eq_attr "isa" "avx10_2") (symbol_ref "TARGET_AVX10_2")
> > @@ -6746,10 +6748,8 @@ (define_insn "*add<mode>_1<nf_name>"
> >    switch (get_attr_type (insn))
> >      {
> >      case TYPE_LEA:
> > -      if (TARGET_APX_NDD && <nf_applied>)
> > -     return "%{nf%} add{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}";
> > -      else
> > -     return "#";
> > +      gcc_assert (!TARGET_APX_NF);
> > +      return "#";
> >
> >      case TYPE_INCDEC:
> >        if (operands[2] == const1_rtx)
> > @@ -6776,7 +6776,7 @@ (define_insn "*add<mode>_1<nf_name>"
> >                    : "<nf_prefix>add{<imodesuffix>}\t{%2, %0|%0, %2}";
> >      }
> >  }
> > -  [(set_attr "isa" "*,*,*,*,*,apx_ndd,apx_ndd,apx_ndd")
> > +  [(set_attr "isa" "*,*,*,*,noapx_nf,apx_ndd,apx_ndd,apx_ndd")
> >     (set (attr "type")
> >       (cond [(eq_attr "alternative" "4")
> >                (const_string "lea")
>
> I thought we want some r <- r, e apx_ndd alternative (r <- r, l is already
> covered), but then the operands are commutative with % and there is that
> r <- rje, r alternative.  On the other side, the canonical order for
> CONST_INT operand in addition is that CONST_INT is the second operand, not
> the first one, so maybe we do want that.  But unsure where exactly the
> actual insn wants the immediate.  Though given that the TYPE_LEA
> "%{nf%} add{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}" case supposedly
> assembled as well as that r <- rje, r, maybe it accepts immediates in both
> spots.
> But gas rejects the second insn in
> addq    $24, %rax, %rdx
> addq    %rax, $24, %rdx
> so I really wonder how the r <-> rje, r alternative actually works.  This is
> a mess.

Er, "je" is:

(define_memory_constraint "je"
  "@internal Memory operand for APX EVEX-encoded ADD (i.e. APX NDD/NF)."
  (match_operand 0 "apx_evex_add_memory_operand"))

the constraint doesn't accept immediates, so everything is OK on this front.

Uros.