[PATCH] D129207: [X86] isTargetShuffleEquivalent - attempt to match SM_SentinelZero shuffle mask elements using known bits

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:11790
       continue;
+    if (MaskIdx == SM_SentinelZero && 0 <= ExpectedIdx) {
+      // If we need this expected index to be a zero element, then update the
----------------
deadalnix wrote:
> `ExpectedIdx >= 0` ? I find Yoda style conditions to be a bit confusing.
Sure - no preference tbh (and I'm never very consistent either).


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:11812
   }
-  return true;
+  return (ZeroV1.isNullValue() || DAG.MaskedVectorIsZero(V1, ZeroV1)) &&
+         (ZeroV2.isNullValue() || DAG.MaskedVectorIsZero(V2, ZeroV2));
----------------
deadalnix wrote:
> Wouldn't that make sense to check for isNullValue in MaskedVectorIsZero?
Doesn't work unfortunately (I agree its tidier), value tracking tends to have things like this:
```
  KnownBits Known(BitWidth);   // Don't know anything
  ...
  if (!DemandedElts)
    return Known;  // No demanded elts, better to assume we don't know anything.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129207