[PATCH] D48025: [PowerPC] avoid masking already-zero bits in BitPermutationSelector

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

Seems like a good idea. A couple of questions below.



================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:1319
       if (V.getValueType() != MVT::i64 ||
-          V.getOperand(0).getValueType() != MVT::i32)
+          V.getOperand(0).getValueSizeInBits() != 32)
         break;
----------------
Why was this changed? I'm fine with it either way, but I think that we should be consistent. Either change the type for V and V.getOperand(0), or the number of bits for both.


================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:1426
+      // when the first non-zero bit appears later.
+      if (IsGroupOfZeros && Bits[i].isZero())
+        continue;
----------------
Couldn't you end up here with a single group that's a mixture of constant zeros and known-to-be-zero bits from different underlying values (with, potentially, different rotation factors)? If so, isn't that a problem?


https://reviews.llvm.org/D48025