[SDAG][RISCV] vnsrl pattern is broken after PR93182

[dtcxzyw]

See the diff of file result/rvv/193fc4d8b7725d32.S in dtcxzyw/llvm-codegen-benchmark@c134d37#diff-bc8f186427aa6515f00cd4d764feea719d5d90240f5602f6a8acf2e114cc9660

; llc -mtriple=riscv64 -mattr=+v test.ll -debug -o -
define <8 x i16> @func0000000000000000(<8 x i32> %0) #0 {
entry:
  %1 = sdiv <8 x i32> %0, <i32 64, i32 64, i32 64, i32 64, i32 64, i32 64, i32 64, i32 64>
  %2 = trunc <8 x i32> %1 to <8 x i16>
  %3 = shl <8 x i16> %2, <i16 10, i16 10, i16 10, i16 10, i16 10, i16 10, i16 10, i16 10>
  ret <8 x i16> %3
}

Before:

        vsetivli zero, 8, e32, m2, ta, ma
        vsra.vi v10, v8, 31
        vsrl.vi v10, v10, 26
        vadd.vv v8, v8, v10
        vsetvli zero, zero, e16, m1, ta, ma
        vnsrl.wi        v10, v8, 10
        vsll.vi v8, v10, 6
        ret

After #93182:

        vsetivli zero, 8, e32, m2, ta, ma
        vsra.vi v10, v8, 31
        vsrl.vi v10, v10, 26
        vadd.vv v8, v8, v10
        vsetvli zero, zero, e16, m1, ta, ma
        vnsrl.wi        v10, v8, 0
        vsll.vi v8, v10, 4
        li      a0, -1024
        vand.vx v8, v8, a0
        ret

I think the following code breaks vnsrl (i.e., trunc (srl X, C)) pattern:

llvm-project/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

Lines 2609 to 2618 in 2ee7f49

	if (!(HighBits & DemandedBits)) {
	// None of the shifted in bits are needed. Add a truncate of the
	// shift input, then shift it.
	SDValue NewShAmt =
	TLO.DAG.getShiftAmountConstant(ShVal, VT, dl, TLO.LegalTypes());
	SDValue NewTrunc =
	TLO.DAG.getNode(ISD::TRUNCATE, dl, VT, Src.getOperand(0));
	return TLO.CombineTo(
	Op, TLO.DAG.getNode(ISD::SRL, dl, VT, NewTrunc, NewShAmt));
	}

Do we need a TLI hook to query whether (trunc (srl X, C)) is natively supported?

cc @RKSimon @topperc

[llvmbot]

@llvm/issue-subscribers-backend-risc-v

Author: Yingwei Zheng (dtcxzyw)

See the diff of file `result/rvv/193fc4d8b7725d32.S` in https://github.com/dtcxzyw/llvm-codegen-benchmark/commit/c134d37047d3e6ee2336279936f98a6eab922e77#diff-bc8f186427aa6515f00cd4d764feea719d5d90240f5602f6a8acf2e114cc9660

; llc -mtriple=riscv64 -mattr=+v test.ll -debug -o -
define &lt;8 x i16&gt; @<!-- -->func0000000000000000(&lt;8 x i32&gt; %0) #<!-- -->0 {
entry:
  %1 = sdiv &lt;8 x i32&gt; %0, &lt;i32 64, i32 64, i32 64, i32 64, i32 64, i32 64, i32 64, i32 64&gt;
  %2 = trunc &lt;8 x i32&gt; %1 to &lt;8 x i16&gt;
  %3 = shl &lt;8 x i16&gt; %2, &lt;i16 10, i16 10, i16 10, i16 10, i16 10, i16 10, i16 10, i16 10&gt;
  ret &lt;8 x i16&gt; %3
}

Before:

        vsetivli zero, 8, e32, m2, ta, ma
        vsra.vi v10, v8, 31
        vsrl.vi v10, v10, 26
        vadd.vv v8, v8, v10
        vsetvli zero, zero, e16, m1, ta, ma
        vnsrl.wi        v10, v8, 10
        vsll.vi v8, v10, 6
        ret

After #93182:

        vsetivli zero, 8, e32, m2, ta, ma
        vsra.vi v10, v8, 31
        vsrl.vi v10, v10, 26
        vadd.vv v8, v8, v10
        vsetvli zero, zero, e16, m1, ta, ma
        vnsrl.wi        v10, v8, 0
        vsll.vi v8, v10, 4
        li      a0, -1024
        vand.vx v8, v8, a0
        ret

I think the following code breaks vnsrl (i.e., trunc (srl X, C)) pattern:

llvm-project/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

Lines 2609 to 2618 in 2ee7f49

	if (!(HighBits & DemandedBits)) {
	// None of the shifted in bits are needed. Add a truncate of the
	// shift input, then shift it.
	SDValue NewShAmt =
	TLO.DAG.getShiftAmountConstant(ShVal, VT, dl, TLO.LegalTypes());
	SDValue NewTrunc =
	TLO.DAG.getNode(ISD::TRUNCATE, dl, VT, Src.getOperand(0));
	return TLO.CombineTo(
	Op, TLO.DAG.getNode(ISD::SRL, dl, VT, NewTrunc, NewShAmt));
	}

Do we need a TLI hook to query whether (trunc (srl X, C)) is natively supported?

cc @RKSimon @topperc

[RKSimon]

Do we need a TLI hook to query whether (trunc (srl X, C)) is natively supported?

There is a isTypeDesirableForOp check after type legalization - is that not working?

[dtcxzyw]

Do we need a TLI hook to query whether (trunc (srl X, C)) is natively supported?

There is a isTypeDesirableForOp check after type legalization - is that not working?

RISC-V target doesn't override this method. I think we need a new hook like isNarrowingProfitableForOp.

[RKSimon]

Why can't it use isTypeDesirableForOp ?

[dtcxzyw]

Why can't it use isTypeDesirableForOp ?

ISD::SRL is legal for both v8i32 and v8i16 on RISCV with rvv.

[RKSimon]

What about if we updated isTypeDesirableForOp to take NewVT + OldVT arguments?

I always worry when we add yet another TLI hook :(

[dtcxzyw]

What about if we updated isTypeDesirableForOp to take NewVT + OldVT arguments?

It sounds reasonable.

[Fros1er]

May I get assigned to this issue? I'm guided by @dtcxzyw.