[PATCH] D124903: [X86][AMX] Replace PXOR instruction with SET0 in AMX pre config.

Tue May 3 22:30:21 PDT 2022

LuoYuanke added inline comments.

================
Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:393
         .addReg(Ymm);
   } else {
     assert(ST.hasSSE2() && "AMX should assume SSE2 enabled");
----------------
craig.topper wrote:
> LuoYuanke wrote:
> > LuoYuanke wrote:
> > > craig.topper wrote:
> > > > LuoYuanke wrote:
> > > > > craig.topper wrote:
> > > > > > Why is there no AVX1 code path to use VMOVUPSmr for 128 bits?
> > > > > I think usaully AMX imply AVX512 enabled, and for the correctness, it can be covered by SSE2 instruction MOVUPSmr. So I think AVX1 code path is nice to have, but not must.
> > > > We usually try not to mix AVX and SSE code due to the impact to the upper bits and how it affects the vzeroupper insertion. But I guess since these are stores they don't dirty the upper bits so maybe it isn't an impact.
> > > Thanks for reminding. Actually I'm going to ask why use VMOVUPSmr for 128 bits given MOVUPSmr works both for SEE and AVX. So AVX-SSE switch penatly is a reason.
> > @craig.topper, do you prefer adding AVX1 code path?
> It's more consistent with other code. I wouldn't do it in this patch though.
OK, I'll add the AVX1 code path.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124903/new/

https://reviews.llvm.org/D124903