[PATCH] D108093: [X86] Support avx512fp16 compare instructions in the IntelInstPrinter.

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp:204-208
+              (Desc.TSFlags & X86II::OpPrefixMask) == X86II::XS)
+            printwordmem(MI, CurOp--, OS);
+          else if ((Desc.TSFlags & X86II::OpMapMask) == X86II::TA &&
+                   (Desc.TSFlags & X86II::OpPrefixMask) == X86II::XD)
+            llvm_unreachable("Unexpected op prefix!");
----------------
pengfei wrote:
> pengfei wrote:
> > craig.topper wrote:
> > > pengfei wrote:
> > > > Why don't use assert like above?
> > > TA can also have PS and/or PD which need to got to EVEX_L2/VEX_L. So I didn't see a clean way to do it. Feel free to make a concrete suggestion.
> > How about:
> > ```
> > if ((Desc.TSFlags & X86II::OpPrefixMask) == X86II::XS) {
> >   if ((Desc.TSFlags & X86II::OpMapMask) == X86II::TA)
> >     printwordmem(MI, CurOp--, OS);
> >   else
> >     printdwordmem(MI, CurOp--, OS);
> > } else if ((Desc.TSFlags & X86II::OpPrefixMask) == X86II::XD) {
> >   assert((Desc.TSFlags & X86II::OpMapMask) != X86II::TA) &&
> >          "Unexpected op map!");
> >   printqwordmem(MI, CurOp--, OS);
> > } ...
> > ```
> Sorry for the noise, I missed the EVEX_L2/VEX_L.
Emm, seems correct :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108093