[PATCH] D144559: [RISCV] XTHeadMemPair: Fix invalid mempair combine for types other than i32/i64

[Manolis Tsamis via Phabricator via llvm-commits]

mtsamis created this revision.
mtsamis added a reviewer: craig.topper.
Herald added subscribers: luke, VincentWu, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya, arichardson.
Herald added a project: All.
mtsamis requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

A mistake in the control flow of performMemPairCombine resulted in paired
loads/stores for types that were not supported by the instructions (i8/i16).
These loads/stores could not match the constraints of the patterns defined
in the THead td file and the compiler would throw a 'Cannot select' error.

This is now fixed and two new test functions have been added in xtheadmempair.ll
which would previously crash the compiler. The compiler was additionally tested
with a wide range of benchmarks and no issues were observed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144559

Files:
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/test/CodeGen/RISCV/xtheadmempair.ll


Index: llvm/test/CodeGen/RISCV/xtheadmempair.ll
===================================================================
--- llvm/test/CodeGen/RISCV/xtheadmempair.ll
+++ llvm/test/CodeGen/RISCV/xtheadmempair.ll
@@ -293,3 +293,51 @@
   store i128 %b, i128* %1, align 8
   ret void
 }
+
+define i32 @lh(i16* %a) {
+; RV32XTHEADMEMPAIR-LABEL: lh:
+; RV32XTHEADMEMPAIR:       # %bb.0:
+; RV32XTHEADMEMPAIR-NEXT:    lh a1, 0(a0)
+; RV32XTHEADMEMPAIR-NEXT:    lh a0, 2(a0)
+; RV32XTHEADMEMPAIR-NEXT:    add a0, a1, a0
+; RV32XTHEADMEMPAIR-NEXT:    ret
+;
+; RV64XTHEADMEMPAIR-LABEL: lh:
+; RV64XTHEADMEMPAIR:       # %bb.0:
+; RV64XTHEADMEMPAIR-NEXT:    lh a1, 0(a0)
+; RV64XTHEADMEMPAIR-NEXT:    lh a0, 2(a0)
+; RV64XTHEADMEMPAIR-NEXT:    add a0, a1, a0
+; RV64XTHEADMEMPAIR-NEXT:    ret
+  %1 = getelementptr i16, i16* %a, i64 0
+  %2 = load i16, i16* %1, align 4
+  %3 = getelementptr i16, i16* %a, i64 1
+  %4 = load i16, i16* %3, align 4
+  %5 = sext i16 %2 to i32
+  %6 = sext i16 %4 to i32
+  %7 = add i32 %5, %6
+  ret i32 %7
+}
+
+define i32 @lb(i8* %a) {
+; RV32XTHEADMEMPAIR-LABEL: lb:
+; RV32XTHEADMEMPAIR:       # %bb.0:
+; RV32XTHEADMEMPAIR-NEXT:    lb a1, 0(a0)
+; RV32XTHEADMEMPAIR-NEXT:    lb a0, 1(a0)
+; RV32XTHEADMEMPAIR-NEXT:    add a0, a1, a0
+; RV32XTHEADMEMPAIR-NEXT:    ret
+;
+; RV64XTHEADMEMPAIR-LABEL: lb:
+; RV64XTHEADMEMPAIR:       # %bb.0:
+; RV64XTHEADMEMPAIR-NEXT:    lb a1, 0(a0)
+; RV64XTHEADMEMPAIR-NEXT:    lb a0, 1(a0)
+; RV64XTHEADMEMPAIR-NEXT:    add a0, a1, a0
+; RV64XTHEADMEMPAIR-NEXT:    ret
+  %1 = getelementptr i8, i8* %a, i64 0
+  %2 = load i8, i8* %1, align 4
+  %3 = getelementptr i8, i8* %a, i64 1
+  %4 = load i8, i8* %3, align 4
+  %5 = sext i8 %2 to i32
+  %6 = sext i8 %4 to i32
+  %7 = add i32 %5, %6
+  ret i32 %7
+}
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -9774,16 +9774,20 @@
         continue;
 
       // Check if the offsets match the XTHeadMemPair encoding contraints.
+      bool valid = false;
       if (MemVT == MVT::i32) {
         // Check for adjacent i32 values and a 2-bit index.
-        if ((Offset1 + 4 != Offset2) || !isShiftedUInt<2, 3>(Offset1))
-          continue;
+        if ((Offset1 + 4 == Offset2) && isShiftedUInt<2, 3>(Offset1))
+          valid = true;
       } else if (MemVT == MVT::i64) {
         // Check for adjacent i64 values and a 2-bit index.
-        if ((Offset1 + 8 != Offset2) || !isShiftedUInt<2, 4>(Offset1))
-          continue;
+        if ((Offset1 + 8 == Offset2) && isShiftedUInt<2, 4>(Offset1))
+          valid = true;
       }
 
+      if (!valid)
+        continue;
+
       // Try to combine.
       if (SDValue Res =
               tryMemPairCombine(DAG, LSNode1, LSNode2, Base1, Offset1))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144559.499467.patch
Type: text/x-patch
Size: 2872 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230222/e13cc352/attachment.bin>