[llvm] [GISel][RISCV] Fix several boundary cases in narrow G_SEXT_INREG. (PR #72719)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 17 15:36:02 PST 2023


https://github.com/topperc created https://github.com/llvm/llvm-project/pull/72719

This fixes cases when SizeInBits is a multiple of the narrow size.

If SizeBits is equal to NarrowTy size, the first block would create an illegal G_SEXT_INREG where the the extension size is equal to the type. I tried to turn it into G_TRUNC+G_SEXT, but that just turned back into G_SEXT_INREG causing an infinite loop. So punt to the splitting case.

In the for loop we should copy when the part ends on SizeInBits. In that case there is no G_SEXT_INREG needed for partial. But we should note that register in PartialExtensionReg for the first full part to use.

If the part starts on SizeInBits then we should do an AShr of PartialExtensionReg.

We should only get to the G_SEXT_INREG case if the SizeInBits is in the middle of the part.

>From 4490e22670d8ae7fcc5ad6535421391ac5579da6 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 17 Nov 2023 15:19:45 -0800
Subject: [PATCH] [GISel][RISCV] Fix several boundary cases in narrow
 G_SEXT_INREG.

This fixes cases when SizeInBits is a multiple of the narrow size.

If SizeBits is equal to NarrowTy size, the first block would create
an illegal G_SEXT_INREG where the the extension size is equal to the type.
I tried to turn it into G_TRUNC+G_SEXT, but that just turned back
into G_SEXT_INREG causing an infinite loop. So punt to the splitting case.

In the for loop we should copy when the part ends on SizeInBits. In
that case there is no G_SEXT_INREG needed for partial. But we should
note that register in PartialExtensionReg for the first full part to
use.

If the part starts on SizeInBits then we should do an AShr of
PartialExtensionReg.

We should only get to the G_SEXT_INREG case if the SizeInBits is in
the middle of the part.
---
 llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp   | 11 ++++++-----
 .../GlobalISel/legalizer/rv32/legalize-mulo.mir   | 15 ++++++---------
 .../GlobalISel/legalizer/rv64/legalize-mulo.mir   | 15 ++++++---------
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index dd5577d47f97764..195f0d60ca10ba2 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -1445,7 +1445,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
 
     // So long as the new type has more bits than the bits we're extending we
     // don't need to break it apart.
-    if (NarrowTy.getScalarSizeInBits() >= SizeInBits) {
+    if (NarrowTy.getScalarSizeInBits() > SizeInBits) {
       Observer.changingInstr(MI);
       // We don't lose any non-extension bits by truncating the src and
       // sign-extending the dst.
@@ -1488,14 +1488,15 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI,
     Register AshrCstReg =
         MIRBuilder.buildConstant(NarrowTy, NarrowTy.getScalarSizeInBits() - 1)
             .getReg(0);
-    Register FullExtensionReg = 0;
-    Register PartialExtensionReg = 0;
+    Register FullExtensionReg;
+    Register PartialExtensionReg;
 
     // Do the operation on each small part.
     for (int i = 0; i < NumParts; ++i) {
-      if ((i + 1) * NarrowTy.getScalarSizeInBits() < SizeInBits)
+      if ((i + 1) * NarrowTy.getScalarSizeInBits() <= SizeInBits) {
         DstRegs.push_back(SrcRegs[i]);
-      else if (i * NarrowTy.getScalarSizeInBits() > SizeInBits) {
+        PartialExtensionReg = DstRegs.back();
+      } else if (i * NarrowTy.getScalarSizeInBits() >= SizeInBits) {
         assert(PartialExtensionReg &&
                "Expected to visit partial extension before full");
         if (FullExtensionReg) {
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mulo.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mulo.mir
index 1b584e0561c77f8..d0929fd19eb92ab 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mulo.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv32/legalize-mulo.mir
@@ -168,16 +168,13 @@ body:             |
     ; LIBCALL-NEXT: ADJCALLSTACKUP 0, 0, implicit-def $x2, implicit $x2
     ; LIBCALL-NEXT: [[COPY2:%[0-9]+]]:_(s32) = COPY $x10
     ; LIBCALL-NEXT: [[COPY3:%[0-9]+]]:_(s32) = COPY $x11
-    ; LIBCALL-NEXT: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
-    ; LIBCALL-NEXT: [[SHL:%[0-9]+]]:_(s32) = G_SHL [[COPY2]], [[C2]](s32)
-    ; LIBCALL-NEXT: [[ASHR2:%[0-9]+]]:_(s32) = G_ASHR [[SHL]], [[C2]](s32)
-    ; LIBCALL-NEXT: [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 31
-    ; LIBCALL-NEXT: [[ASHR3:%[0-9]+]]:_(s32) = G_ASHR [[ASHR2]], [[C3]](s32)
-    ; LIBCALL-NEXT: [[C4:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
-    ; LIBCALL-NEXT: [[XOR:%[0-9]+]]:_(s32) = G_XOR [[COPY2]], [[ASHR2]]
-    ; LIBCALL-NEXT: [[XOR1:%[0-9]+]]:_(s32) = G_XOR [[COPY3]], [[ASHR3]]
+    ; LIBCALL-NEXT: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 31
+    ; LIBCALL-NEXT: [[ASHR2:%[0-9]+]]:_(s32) = G_ASHR [[COPY2]], [[C2]](s32)
+    ; LIBCALL-NEXT: [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 0
+    ; LIBCALL-NEXT: [[XOR:%[0-9]+]]:_(s32) = G_XOR [[COPY2]], [[COPY2]]
+    ; LIBCALL-NEXT: [[XOR1:%[0-9]+]]:_(s32) = G_XOR [[COPY3]], [[ASHR2]]
     ; LIBCALL-NEXT: [[OR:%[0-9]+]]:_(s32) = G_OR [[XOR]], [[XOR1]]
-    ; LIBCALL-NEXT: [[ICMP:%[0-9]+]]:_(s32) = G_ICMP intpred(ne), [[OR]](s32), [[C4]]
+    ; LIBCALL-NEXT: [[ICMP:%[0-9]+]]:_(s32) = G_ICMP intpred(ne), [[OR]](s32), [[C3]]
     ; LIBCALL-NEXT: $x10 = COPY [[COPY2]](s32)
     ; LIBCALL-NEXT: $x11 = COPY [[ICMP]](s32)
     ; LIBCALL-NEXT: PseudoRET implicit $x10, implicit $x11
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-mulo.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-mulo.mir
index 433e1b8e6d577e3..c2bf9ff2a61e9a7 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-mulo.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/rv64/legalize-mulo.mir
@@ -218,16 +218,13 @@ body:             |
     ; LIBCALL-NEXT: ADJCALLSTACKUP 0, 0, implicit-def $x2, implicit $x2
     ; LIBCALL-NEXT: [[COPY2:%[0-9]+]]:_(s64) = COPY $x10
     ; LIBCALL-NEXT: [[COPY3:%[0-9]+]]:_(s64) = COPY $x11
-    ; LIBCALL-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
-    ; LIBCALL-NEXT: [[SHL:%[0-9]+]]:_(s64) = G_SHL [[COPY2]], [[C2]](s64)
-    ; LIBCALL-NEXT: [[ASHR2:%[0-9]+]]:_(s64) = G_ASHR [[SHL]], [[C2]](s64)
-    ; LIBCALL-NEXT: [[C3:%[0-9]+]]:_(s64) = G_CONSTANT i64 63
-    ; LIBCALL-NEXT: [[ASHR3:%[0-9]+]]:_(s64) = G_ASHR [[ASHR2]], [[C3]](s64)
-    ; LIBCALL-NEXT: [[C4:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
-    ; LIBCALL-NEXT: [[XOR:%[0-9]+]]:_(s64) = G_XOR [[COPY2]], [[ASHR2]]
-    ; LIBCALL-NEXT: [[XOR1:%[0-9]+]]:_(s64) = G_XOR [[COPY3]], [[ASHR3]]
+    ; LIBCALL-NEXT: [[C2:%[0-9]+]]:_(s64) = G_CONSTANT i64 63
+    ; LIBCALL-NEXT: [[ASHR2:%[0-9]+]]:_(s64) = G_ASHR [[COPY2]], [[C2]](s64)
+    ; LIBCALL-NEXT: [[C3:%[0-9]+]]:_(s64) = G_CONSTANT i64 0
+    ; LIBCALL-NEXT: [[XOR:%[0-9]+]]:_(s64) = G_XOR [[COPY2]], [[COPY2]]
+    ; LIBCALL-NEXT: [[XOR1:%[0-9]+]]:_(s64) = G_XOR [[COPY3]], [[ASHR2]]
     ; LIBCALL-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR [[XOR]], [[XOR1]]
-    ; LIBCALL-NEXT: [[ICMP:%[0-9]+]]:_(s64) = G_ICMP intpred(ne), [[OR]](s64), [[C4]]
+    ; LIBCALL-NEXT: [[ICMP:%[0-9]+]]:_(s64) = G_ICMP intpred(ne), [[OR]](s64), [[C3]]
     ; LIBCALL-NEXT: $x10 = COPY [[COPY2]](s64)
     ; LIBCALL-NEXT: $x11 = COPY [[ICMP]](s64)
     ; LIBCALL-NEXT: PseudoRET implicit $x10, implicit $x11



More information about the llvm-commits mailing list