[GISel] Convert zext nneg to sext if it is cheaper

[dtcxzyw]

The logic is copied from SelectionDAGBuilder:

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

Lines 3829 to 3836 in 32546bd

	// Eagerly use nonneg information to canonicalize towards sign_extend if
	// that is the target's preference.
	// TODO: Let the target do this later.
	if (Flags.hasNonNeg() &&
	TLI.isSExtCheaperThanZExt(N.getValueType(), DestVT)) {
	setValue(&I, DAG.getNode(ISD::SIGN_EXTEND, getCurSDLoc(), DestVT, N));
	return;
	}

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

Author: Yingwei Zheng (dtcxzyw)

Changes

The logic is copied from SelectionDAGBuilder:

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

Lines 3829 to 3836 in 32546bd

	// Eagerly use nonneg information to canonicalize towards sign_extend if
	// that is the target's preference.
	// TODO: Let the target do this later.
	if (Flags.hasNonNeg() &&
	TLI.isSExtCheaperThanZExt(N.getValueType(), DestVT)) {
	setValue(&I, DAG.getNode(ISD::SIGN_EXTEND, getCurSDLoc(), DestVT, N));
	return;
	}

---

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

3 Files Affected:

• (modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+11)
• (modified) llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip-rv64.ll (+10)
• (added) llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/zext-nneg-rv64.ll (+18)

diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 5289b993476db..d5138c68218c1 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -1569,6 +1569,17 @@ bool IRTranslator::translateCast(unsigned Opcode, const User &U,
 
   Register Op = getOrCreateVReg(*U.getOperand(0));
   Register Res = getOrCreateVReg(U);
+
+  // Convert zext nneg to sext if it is preferred form for the target.
+  if (Opcode == TargetOpcode::G_ZEXT && (Flags & MachineInstr::NonNeg)) {
+    EVT SrcVT = TLI->getValueType(*DL, U.getOperand(0)->getType());
+    EVT DstVT = TLI->getValueType(*DL, U.getType());
+    if (TLI->isSExtCheaperThanZExt(SrcVT, DstVT)) {
+      Opcode = TargetOpcode::G_SEXT;
+      Flags = 0;
+    }
+  }
+
   MIRBuilder.buildInstr(Opcode, {Res}, {Op}, Flags);
   return true;
 }
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip-rv64.ll b/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip-rv64.ll
index d4acca17930d5..fd80afce6510e 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip-rv64.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/alu-roundtrip-rv64.ll
@@ -101,3 +101,13 @@ entry:
   %0 = urem i64 %a, %b
   ret i64 %0
 }
+
+define i64 @zext_nneg_i32_i64(i32 %a) {
+; RV64IM-LABEL: zext_nneg_i32_i64:
+; RV64IM:       # %bb.0: # %entry
+; RV64IM-NEXT:    sext.w a0, a0
+; RV64IM-NEXT:    ret
+entry:
+  %b = zext nneg i32 %a to i64
+  ret i64 %b
+}
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/zext-nneg-rv64.ll b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/zext-nneg-rv64.ll
new file mode 100644
index 0000000000000..2e37da0c03a51
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/zext-nneg-rv64.ll
@@ -0,0 +1,18 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=riscv64 -global-isel -stop-after=irtranslator -verify-machineinstrs < %s \
+; RUN:   | FileCheck -check-prefix=RV64I %s
+
+define i64 @zext_nneg_i32_i64(i32 %a) {
+  ; RV64I-LABEL: name: zext_nneg_i32_i64
+  ; RV64I: bb.1.entry:
+  ; RV64I-NEXT:   liveins: $x10
+  ; RV64I-NEXT: {{  $}}
+  ; RV64I-NEXT:   [[COPY:%[0-9]+]]:_(s64) = COPY $x10
+  ; RV64I-NEXT:   [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[COPY]](s64)
+  ; RV64I-NEXT:   [[SEXT:%[0-9]+]]:_(s64) = G_SEXT [[TRUNC]](s32)
+  ; RV64I-NEXT:   $x10 = COPY [[SEXT]](s64)
+  ; RV64I-NEXT:   PseudoRET implicit $x10
+entry:
+  %b = zext nneg i32 %a to i64
+  ret i64 %b
+}

[tschuett]

Why is the DAG doing this transform in the builder and not in the combiner?

def nneg_zext : GICombineRule<
   (defs root:$root, build_fn_matchinfo:$matchinfo),
   (match (G_ZEXT $root, $x, (MIFlags NonNeg)),
   [{ return Helper.matchNonNegZext(${root}, ${matchinfo}); }]),
   (apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

[dtcxzyw]

Why is the DAG doing this transform in the builder and not in the combiner?

def nneg_zext : GICombineRule<
   (defs root:$root, build_fn_matchinfo:$matchinfo),
   (match (G_ZEXT $root, $x, (MIFlags NonNeg)),
   [{ return Helper.matchNonNegZext(${root}, ${matchinfo}); }]),
   (apply [{ Helper.applyBuildFnMO(${root}, ${matchinfo}); }])>;

@topperc

[arsenm]

I really do not like these optimizations getting snuck into the DAG builder / IRTranslator. They should do their best to preserve the original IR

[dtcxzyw]

I really do not like these optimizations getting snuck into the DAG builder / IRTranslator. They should do their best to preserve the original IR

Alright, I will post an alternative patch as @tschuett suggested.

[tschuett]

The pattern goes in Combine.td. The C++ goes into CombinerHelper. Ping me.

[dtcxzyw]

Closing in favor of #93856.