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).
Jakub
