Refine logic of MaskedElementsAreZero

[xiangzhangllvm]

Summary

The Mask (DemandedBits ) passed to MaskedValueIsZero in MaskedElementsAreZero seems didn't make sense.
Try to refine it at first.

[xiangzhangllvm]

Hello @RKSimon , I add you here, just because I git blamed that the code is your contribution.

Here I want to discuss with you about the logic of MaskedElementsAreZero first before I change and add related tests.

We want to Masked Elements (of vector) Are Zero, so the Mask pass to MaskedValueIsZero (in your old code) should be A Mask of vector.
Not clear why it just "APInt::getAllOnesValue(Op.getScalarValueSizeInBits())"

[RKSimon]

Please do you have a test case to reproduce the issue you encountered?

MaskedElementsAreZero should only be called for vector types, although I failed to add an assert to ensure that.

The SelectionDAG valuetracking code doesn't handle individual vector elements, it just analyses the common bits of all the vector elements specified in the DemandedElts mask - if you really need to determine the known bits of every vector element you need to call computeKnownBits for every element, demanding just that element index.

[xiangzhangllvm]

Yes, I change here because we got a runfail at a project. Let me commit the small reproduce a little later.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2450

it just analyses the common bits of all the vector elements specified in the DemandedElts mask

Thanks for your explain, in fact, I read the computeKnownBits code yesterday, it makes me a lot of puzzle. I thought here the DemandedBits should have the same BitWidth with Op (e.g. 128 for V16xi8)

Let me take a example:
So, if the Op is type V16xi8, and the DemandedElts is 0x2222 (16 bits, demanded index is 1, 5, 9, 13), you mean computeKnownBits will return the common zeros/ones of Op's elements with index 1, 5, 9, 13 ? ( zeros/ones = element1 & element5 & element9 & element13).

[xiangzhangllvm]

Let me abandon it first, your patch self looks no problem.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2450

Yes the KnownBits result is the common bits in those demanded elements - if any are set differently or unknown in any element then the KnownBits result doesn't 'know' that bit.

If you have a usecase for a 'computeAllKnownBits' style wrapper that returns the known bits for each element (either as an array of KnownBits or a concatenated KnownBits struct) then it'd be definitely worth a patch along with suitable test coverage.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

bool SelectionDAG::MaskedValueIsZero(SDValue V, const APInt &Mask,
                                      unsigned Depth) const {
   return Mask.isSubsetOf(computeKnownBits(V, DemandedElts, Depth).Zero);
 }
 
 /// Return true if the DemandedElts of the vector Op are all zero.  We
 /// use this predicate to simplify operations downstream.
 bool SelectionDAG::MaskedElementsAreZero(SDValue Op, const APInt &DemandedElts,
                                          unsigned Depth) const {
+  APInt DemandedBits;
+  EVT VT = Op.getValueType();
   unsigned BitWidth = Op.getScalarValueSizeInBits();
-  APInt DemandedBits = APInt::getAllOnesValue(BitWidth);
-  return MaskedValueIsZero(Op, DemandedBits, DemandedElts, Depth);
+
+  if (VT.isVector()) {
+    unsigned NumElts = VT.getVectorNumElements();
+    assert(NumElts == DemandedElts.getBitWidth() && "Unexpected vector size");
+    DemandedBits = APInt::getNullValue(BitWidth * NumElts);
+    for (int i = 0; i < NumElts; i++) {
+      if (!DemandedElts[i])
+        continue;
+      DemandedBits.setBits(BitWidth * i, BitWidth * (i + 1));
+    }
+  } else {
+    DemandedBits = APInt::getAllOnesValue(BitWidth);
+  }
+
+  APInt Zero = computeKnownBits(Op, DemandedElts, Depth).Zero;
+
+  if (DemandedBits.getBitWidth() > Zero.getBitWidth())
+    Zero = Zero.zext(DemandedBits.getBitWidth());
+  else
+    Zero = Zero.trunc(DemandedBits.getBitWidth());
+
+  return DemandedBits.isSubsetOf(Zero);
 }
 
 /// MaskedValueIsAllOnes - Return true if '(Op & Mask) == Mask'.
 bool SelectionDAG::MaskedValueIsAllOnes(SDValue V, const APInt &Mask,
                                         unsigned Depth) const {
   return Mask.isSubsetOf(computeKnownBits(V, Depth).One);
 }