On Thu, May 28, 2026 at 12:13 PM Jakub Jelinek <[email protected]> wrote:
>
> On Thu, May 28, 2026 at 11:58:37AM +0200, Uros Bizjak wrote:
> > On Thu, May 28, 2026 at 11:48 AM Jakub Jelinek <[email protected]> wrote:
> > >
> > > On Thu, May 28, 2026 at 11:31:27AM +0200, Uros Bizjak wrote:
> > > > Hm, OTOH, is this condition correct:
> > > >
> > > > case TYPE_LEA:
> > > > if (TARGET_APX_NDD && <nf_applied>)
> > > > return "%{nf%} add{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}";
> > > > else
> > > > return "#";
> > > >
> > > > Should it be "TARGET_APX_NDD || <nf_applied>"?
> > >
> > > The pattern is weird, but TARGET_APX_NDD || <nf_applied> would be wrong.
> > > Without TARGET_APX_NDD there is no 3 operand add, so we can't use that.
> > > Of course, question is if there will be CPUs with just APX-NF and not
> > > APX-NDD or with just APX-NDD and not APX-NF.
> > >
> > > I'd say it is a bad idea to have the "lea" variant come before the
> > > 3 apx_ndd variants, if the lea variant came last, then at least the r <-
> > > r, l
> > > case would be covered for TARGET_APX_NDD by the earlier (but now later)
> > > r <- rje, r variant and could then emit either the 3 operand add or 3
> > > operand add with {nf}. Anyway, with the current position it could also be
> > > if (TARGET_APX_NDD)
> > > return "<nf_prefix>add{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}";
> > > else
> > > {
> > > operands[3] = gen_rtx_PLUS (<MODE>mode, operands[1], operands[2]);
> > > return "lea{<imodesuffix>}\t{%E3, %0|%0, %E3}";
> > > }
> >
> > I think that the correct solution is to put LEA at the end. LEA
> > alternative is intended for non-NDD cases to allow two-register
> > operation, but NDD should override it, w/ or w/o NF. Let's ask
> > Hongtao.
>
> Note, in GCC 13 we had:
>
> (define_insn "*add<mode>_1"
> [(set (match_operand:SWI48 0 "nonimmediate_operand" "=rm,r,r,r")
> (plus:SWI48
> (match_operand:SWI48 1 "nonimmediate_operand" "%0,0,r,r")
> (match_operand:SWI48 2 "x86_64_general_operand" "re,BM,0,le")))
> (clobber (reg:CC FLAGS_REG))]
> "ix86_binary_operator_ok (PLUS, <MODE>mode, operands)"
> {
> switch (get_attr_type (insn))
> {
> case TYPE_LEA:
> return "#";
> ...
> )
> and
> ;; Convert add to the lea pattern to avoid flags dependency.
> (define_split
> [(set (match_operand:SWI 0 "register_operand")
> (plus:SWI (match_operand:SWI 1 "register_operand")
> (match_operand:SWI 2 "<nonmemory_operand>")))
> (clobber (reg:CC FLAGS_REG))]
> "reload_completed && ix86_lea_for_add_ok (insn, operands)"
> [(set (match_dup 0)
> (plus:<LEAMODE> (match_dup 1) (match_dup 2)))]
> (and others for ix86_avoid_lea*).
>
> That is fine, the only problem is the <nf_applied> lea case.
> So perhaps I should change the committed patch to be
> if (TARGET_APX_NDD && <nf_applied>)
> return "%{nf%} add{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}";
> else if (<nf_applied>)
> {
> operands[3] = gen_rtx_PLUS (<MODE>mode, operands[1], operands[2]);
> return "lea{<imodesuffix>}\t{%E3, %0|%0, %E3}";
> }
> else
> return "#";
> to restore previous behavior for the non-nf case. Or the lea variant
> should be just disabled in the <nf_applied> case, rely on *lea<mode>
> being matched instead (which comes earlier).
Like the attached patch?
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")
