[DAGCombiner][RISCV] Prefer to sext i32 non-negative values

[dtcxzyw]

By default, DAGCombiner folds sext x to zext x when x is non-negative. It will generate redundant zext inst seq on riscv64 (typically slli (srli x, 32), 32).
godbolt: https://godbolt.org/z/osf6adP1o
This patch applies the transform iff zext is cheaper than sext.

[RKSimon]

LGTM

[topperc]

InstCombine does the same thing. Do we need a reverse transform to turn zext into sext explicitly? Or perhaps an isel pattern?

[dtcxzyw]

InstCombine does the same thing.

I think you mean ISel.

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

Line 6072 in 231aa0f

// Some targets like RISCV prefer to sign extend some types.

Do we need a reverse transform to turn zext into sext explicitly? Or perhaps an isel pattern?

It is unsafe to turn zext into sext. Why not just prevent tuning zext into sext?

[topperc]

InstCombine does the same thing.

I think you mean ISel.

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

Line 6072 in 231aa0f

// Some targets like RISCV prefer to sign extend some types.

Do we need a reverse transform to turn zext into sext explicitly? Or perhaps an isel pattern?

It is unsafe to turn zext into sext. Why not just prevent tuning zext into sext?

visitSExt in InstCombine will turn sext into zext. That code will happen before DAGCombine. The DAGCombine change will only effect sext that is created during SelectionDAG. sext that already existed in IR may get changed by InstCombine.

So my question was do we need to change zext to sext if the sign bit of the input is known to be 0 by computeKnownBits.

[dtcxzyw]

llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

Lines 1375 to 1377 in c464896

	// If the value being extended is zero or positive, use a zext instead.
	if (isKnownNonNegative(Src, DL, 0, &AC, &Sext, &DT))
	return CastInst::Create(Instruction::ZExt, Src, DestTy);

[dtcxzyw]

Diff: main...dtcxzyw:llvm-project:zext-to-sext
Evaluation result: dtcxzyw/llvm-ci#627

It seems better to sext a non-negative value on riscv64. But this change broke some regression tests.