[DAGCombiner] Relax an assertion to an early return

[frasercrmck]

Summary

The select-of-constants transform was asserting that its constant vector
inputs did not implicitly truncate their input without that as an
explicit precondition to the function. This patch relaxes that assertion
into an early return to skip the optimization.

[lebedev.ri]

We got here because in foldSelectOfConstants()

auto *C1 = dyn_cast<ConstantSDNode>(N1);
auto *C2 = dyn_cast<ConstantSDNode>(N2);
if (!C1 || !C2)
  return SDValue();

didn't return.
So what are those constants?

[frasercrmck]

In D102393#2756528, @lebedev.ri wrote:

We got here because in foldSelectOfConstants()

auto *C1 = dyn_cast<ConstantSDNode>(N1);
auto *C2 = dyn_cast<ConstantSDNode>(N2);
if (!C1 || !C2)
  return SDValue();

didn't return.
So what are those constants?

They're constants but they have implicit truncation. After legalization, RV64 -- which doesn't have legal i32 -- sees:

t9: v4i32 = BUILD_VECTOR Constant:i64<878082066>, Constant:i64<878082066>, Constant:i64<878082066>, Constant:i64<878082066>
t11: v4i32 = BUILD_VECTOR Constant:i64<1164411171>, Constant:i64<1164411171>, Constant:i64<1164411171>, Constant:i64<1164411171>

[RKSimon]

Have you investigated whether we can improve support for implicit truncation of build vector element on this path instead of just bailing out?

[frasercrmck]

In D102393#2757020, @RKSimon wrote:

Have you investigated whether we can improve support for implicit truncation of build vector element on this path instead of just bailing out?

Yeah I took a look. This transform later implicitly forbids implicit truncation via its use of isAllOnesOrAllOnesSplat and isNullOrNullSplat so that would need changed. I don't see why it couldn't support implicit truncation as long as it doesn't blindly take one of the constant vector elements assuming it's the same type as the resulting vector element type. The question is also whether we can get a test case, given this combine also runs before legalization where we (RISC-V) wouldn't yet have the implicit truncation.

That said, it seems like this restriction on implicit truncation is a common idiom in the DAGCombiner -- given by the number of uses of isConstantOrConstantVector alone -- so maybe that's a larger discussion? There's so much almost-duplication in SelectionDAG for "isBuildVector", "isSplat", "isBuildOrSplat", "isConstantSplat", "isConstantSplatAllOnes", etc., that I don't think anyone wants to start untangling that web.

[RKSimon]

LGTM - cheers

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

 }
 
 /// If a (v)select has a condition value that is a sign-bit test, try to smear
 /// the condition operand sign-bit across the value width and use it as a mask.
 static SDValue foldSelectOfConstantsUsingSra(SDNode *N, SelectionDAG &DAG) {
   SDValue Cond = N->getOperand(0);
   SDValue C1 = N->getOperand(1);
   SDValue C2 = N->getOperand(2);
-  assert(isConstantOrConstantVector(C1) && isConstantOrConstantVector(C2) &&
-         "Expected select-of-constants");
+  if (!isConstantOrConstantVector(C1) || !isConstantOrConstantVector(C2))
+    return SDValue();
 
   EVT VT = N->getValueType(0);
   if (Cond.getOpcode() != ISD::SETCC || !Cond.hasOneUse() ||
       VT != Cond.getOperand(0).getValueType())
     return SDValue();
 
   // The inverted-condition + commuted-select variants of these patterns are
   // canonicalized to these forms in IR.

llvm/test/CodeGen/RISCV/rvv/select-sra.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+experimental-v -riscv-v-vector-bits-min=128 -verify-machineinstrs < %s | FileCheck %s --check-prefix=RV32
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-v -riscv-v-vector-bits-min=128 -verify-machineinstrs < %s | FileCheck %s --check-prefix=RV64
+
+; This test checks a regression in the select-to-sra transform, which was
+; asserting (without a precondition) when the vector constants implicitly
+; truncated their inputs, as we do on RV64.
+define <4 x i32> @vselect_of_consts(<4 x i1> %cc) {
+; RV32-LABEL: vselect_of_consts:
+; RV32:       # %bb.0:
+; RV32-NEXT:    lui a0, 284280
+; RV32-NEXT:    addi a0, a0, 291
+; RV32-NEXT:    vsetivli a1, 4, e32,m1,ta,mu
+; RV32-NEXT:    vmv.v.x v25, a0
+; RV32-NEXT:    lui a0, 214376
+; RV32-NEXT:    addi a0, a0, -2030
+; RV32-NEXT:    vmerge.vxm v8, v25, a0, v0
+; RV32-NEXT:    ret
+;
+; RV64-LABEL: vselect_of_consts:
+; RV64:       # %bb.0:
+; RV64-NEXT:    lui a0, 284280
+; RV64-NEXT:    addiw a0, a0, 291
+; RV64-NEXT:    vsetivli a1, 4, e32,m1,ta,mu
+; RV64-NEXT:    vmv.v.x v25, a0
+; RV64-NEXT:    lui a0, 214376
+; RV64-NEXT:    addiw a0, a0, -2030
+; RV64-NEXT:    vmerge.vxm v8, v25, a0, v0
+; RV64-NEXT:    ret
+  %v = select <4 x i1> %cc, <4 x i32> <i32 878082066, i32 878082066, i32 878082066, i32 878082066>, <4 x i32> <i32 1164411171, i32 1164411171, i32 1164411171, i32 1164411171>
+  ret <4 x i32> %v
+}