[LegalizeVectorOps] Use getBoolConstant instead of getAllOnesConstant in VectorLegalizer::UnrollVSETCC.

[topperc]

This code should follow the target preference for boolean contents of a vector type. We shouldn't assume that true is negative one.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

Author: Craig Topper (topperc)

Changes

This code should follow the target preference for boolean contents of a vector type. We shouldn't assume that true is negative one.

---

Full diff: https://github.com/llvm/llvm-project/pull/121526.diff

1 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp (+2-1)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
index db21e708970648..93e081ec9ccc02 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp
@@ -2250,7 +2250,8 @@ SDValue VectorLegalizer::UnrollVSETCC(SDNode *Node) {
                          TLI.getSetCCResultType(DAG.getDataLayout(),
                                                 *DAG.getContext(), TmpEltVT),
                          LHSElem, RHSElem, CC);
-    Ops[i] = DAG.getSelect(dl, EltVT, Ops[i], DAG.getAllOnesConstant(dl, EltVT),
+    Ops[i] = DAG.getSelect(dl, EltVT, Ops[i],
+                           DAG.getBoolConstant(true, dl, EltVT, VT),
                            DAG.getConstant(0, dl, EltVT));
   }
   return DAG.getBuildVector(VT, dl, Ops);

[RKSimon]

I still think this should be:

    Ops[i] = DAG.getNode(ISD::SETCC, dl, MVT::i1, LHSElem, RHSElem, CC);
    Ops[i] = DAG.getBoolExtOrTrunc(Ops[i], dl, EltVT, VT);

But the last time I attempted this it resulted in a lot of churn: https://github.com/RKSimon/llvm-project/tree/unroll-setcc

[topperc]

Thanks. I didn't know there was history here. Have any of the yaks been shaved yet?

Still seems like we should either fix the code with getBoolConstant or add a FIXME until we can use getBoolExtOrTrunc. That way the next person who reads this code doesn't think it's wrong like I just did.

[RKSimon]

Agreed, a fixme comment would be acceptable for now.

[RKSimon]

LGTM with a suitable comment that we should be avoiding the select and performing a setcc+boolext instead