Thanks for your review! I'll ask Hongtao to check-in the patch for me.
Uros Bizjak <ubiz...@gmail.com> 于2020年10月29日周四 下午4:08写道：
>
> On Thu, Oct 29, 2020 at 7:52 AM Hongyu Wang <wwwhhhyyy...@gmail.com> wrote:
> >
> > Hi Uros,
> >
> > > is there a reason to introduce all these (with corresponding changes)?
> > > SSE options live in ISA bitmap, so it is kind of strange you need to
> > > handle them in ISA2 bitmap. Option handling is not exactly my area,
> > > please ask HJ to comment and review this part.
> >
> > As Hongtao said, this part is needed for new non-avx512 ISAs in ISA2 bitmap.
> > Keylocker is the first one, and we do similar thing in AVX-VNNI.
>
> Thanks for the explanation, LGTM for this part.
>
> > > + pat = gen_rtx_EQ (QImode, gen_rtx_REG (CCZmode, FLAGS_REG),
> > > + const0_rtx);
> > > + emit_move_insn (target, pat);
> > >
> > > emit_insn (gen_rtx_SET (target, pat));
> > >
> >
> > Changed.
> >
> > > + op1 = copy_to_suggested_reg (op1,
> > > + gen_rtx_REG (V2DImode,
> > > + GET_SSE_REGNO (0)),
> > > + V2DImode);
> > > +
> > > + xmm_regs[0] = op1;
> > >
> > > this is no better than:
> > >
> > > reg = gen_rtx_REG (V2DImode, GET_SSE_REGNO (0));
> > > emit_move_insn (reg, op1)
> >
> > Changed.
> >
> > > + xmm_regs[0] = op1;
> > > + for (i = 1; i < 3; i++)
> > > + xmm_regs[i] = gen_rtx_REG (V2DImode, GET_SSE_REGNO (i));
> > >
> > > The first line is dead code, copy_to_suggested reg generated (reg
> > > xmm0) RTX for op1. Just use:
> > >
> > > for (i = 0; i < 3; i++)
> > > xmm_regs[i] = gen_rtx_REG (V2DImode, GET_SSE_REGNO (i));
> > >
> > > Similar comments for:
> >
> > All changed.
> >
> > > + for (i = 0; i < 4; i++)
> > > + {
> > > + tmp_unspec
> > > + = gen_rtx_UNSPEC_VOLATILE (V2DImode,
> > > + gen_rtvec (1, const0_rtx),
> > > + UNSPECV_ENCODEKEY256U32);
> > >
> > > Please move the above out of the loop.
> > Sorry for such code. Changed.
> >
> > > Here lies the reason to use hard registers. "sse_reg_operand" is not
> > > enough for register allocator. We have no constraint for first SSE
> > > reg, so again
> > > use hard registers here instead of operands 2 and 3: (reg:V2DI
> > > XMM0_REG) and (reg:V2DI XMM1_REG).
> >
> > Changed for encodekey128u32, encodekey256u32 expander and
> > patterns, adjust corresponding function call in i386-expand.c.
> >
> > Thanks for all the the helpful comments. Updated patch.
>
> The patch is OK for mainline.
>
> (There are some cleanup opportunities, I'll commit them as a
> no-functional-change patch in stage3.)
>
> Thanks,
> Uros.
--
Regards,
Hongyu, Wang
