[PATCH] D103280: [SDAG] try harder to fold casts into vector compare
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 28 04:48:21 PDT 2021
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
More information about the llvm-commits
mailing list