[PATCH] D145221: [X86] Prefer `vpternlog` instead of `blendv` for `vselect` on masks.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1044
+      if (Subtarget->hasVLX() && Subtarget->hasAVX512() &&
+          N->getOperand(0)->getOpcode() == X86ISD::PCMPGT) {
+        R = CurDAG->getNode(X86ISD::VPTERNLOG, SDLoc(N), N->getValueType(0),
----------------
goldstein.w.n wrote:
> goldstein.w.n wrote:
> > RKSimon wrote:
> > > craig.topper wrote:
> > > > Why is the PCMPGT check needed?
> > > Wouldn't a numsignbits check be better than PCMPGT?
> > > Why is the PCMPGT check needed?
> > 
> > `blendv` only uses the sign-bit of the control. For `vpternlog` to be a replacement, the control needs to be in mask form (elements either -1/0). `pcmpgt` is just a check for "is the control in mask form". Maybe `(sext (setcc ...))` would also work but didn't see any codegen changes from it.
> What is numsignbits check? 
CurDAG->computeNumSignBits or CurDAG->ComputeMaxSignificantBits


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1044
+      if (Subtarget->hasVLX() && Subtarget->hasAVX512() &&
+          N->getOperand(0)->getOpcode() == X86ISD::PCMPGT) {
+        R = CurDAG->getNode(X86ISD::VPTERNLOG, SDLoc(N), N->getValueType(0),
----------------
craig.topper wrote:
> goldstein.w.n wrote:
> > goldstein.w.n wrote:
> > > RKSimon wrote:
> > > > craig.topper wrote:
> > > > > Why is the PCMPGT check needed?
> > > > Wouldn't a numsignbits check be better than PCMPGT?
> > > > Why is the PCMPGT check needed?
> > > 
> > > `blendv` only uses the sign-bit of the control. For `vpternlog` to be a replacement, the control needs to be in mask form (elements either -1/0). `pcmpgt` is just a check for "is the control in mask form". Maybe `(sext (setcc ...))` would also work but didn't see any codegen changes from it.
> > What is numsignbits check? 
> CurDAG->computeNumSignBits or CurDAG->ComputeMaxSignificantBits
I think an ISD::VSELECT is technically malformed if the elements of the condition aren't 0/-1, but we don't have a good way of proving we didn't mess that up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145221