[PATCH] D103280: [SDAG] try harder to fold casts into vector compare

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10946-10947
+          TLI.isOperationLegalOrCustom(ISD::SETCC, VT) &&
+          !TLI.isOperationLegalOrCustom(ISD::SETCC, SVT)) {
+        // We have an unsupported narrow vector compare op that would be legal
+        // if extended to the destination type. See if the compare operands
----------------
pengfei wrote:
> lebedev.ri wrote:
> > `SVT` is the result type of comparison of values with `N00VT` type.
> > Shouldn't this be `!TLI.isOperationLegalOrCustom(ISD::SETCC, N00VT)` ?
> I think this is used for preventing the legal cases of the result type of comparison, e.g. cmp_slt_load_const?
This is a bit tricky. I missed adding a test for this predicate, but this is what I intended.
That check is there to prevent transforming a legal vector op into a wider op because that's not guaranteed to be a win. Let me add a test and explain further.

And I think we do want to check the `SVT` type because that is how target handling for setcc is specified. For example, with AVX512, we have:

```
    for (auto VT : { MVT::v2i1, MVT::v4i1, MVT::v8i1, MVT::v16i1 }) {
      setOperationAction(ISD::SETCC,            VT, Custom);
```

Let me know if I'm misunderstanding it. I'm not finding an example where I can show a difference because other transforms always seem to change the type before we get here.


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

https://reviews.llvm.org/D103280