[PATCH] D130397: [RISCV] Custom type legalize i32 loads by sign extending.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: reames, luismarques, asb, frasercrmck.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, evandro, 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, hiraditya, arichardson.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

The default is to use extload which can become a zextload or
sextload if it is followed by an 'and' or sext_inreg.

Sometimes type legalization will introduce an 'and' from promoting
something like 'srl X, C' and a sext_inreg from from a setcc. The
'and' could be freely folded with the promoted 'srl' by using srliw,
but the sext_inreg can't be folded into a compare. DAG combiner
will see both of these choices and may decide to fold the 'and'
instead of the 'sext_inreg'. This forces the sext_inreg to become
a sext.w.

By picking sextload in the type legalizer we take this choice away.
Looking at spec2006 compiled with Zba and Zbb this appeared to be
net reduction in lines of code in the objdump disassembly output.

This is similar to what we do with i32 add/sub/mul/shl in
type legalization where we always emit a sext_inreg.

There's some followup improvements we could do. For example, folding
(and (sextload X), 0xffffffff) to (zextload X) if the 'and' is the
only user.

I don't plan to merge this to LLVM 15 which branches next week.
Posting to start discussion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130397

Files:
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/test/CodeGen/RISCV/sextw-removal.ll
  llvm/test/CodeGen/RISCV/vec3-setcc-crash.ll


Index: llvm/test/CodeGen/RISCV/vec3-setcc-crash.ll
===================================================================
--- llvm/test/CodeGen/RISCV/vec3-setcc-crash.ll
+++ llvm/test/CodeGen/RISCV/vec3-setcc-crash.ll
@@ -46,7 +46,7 @@
 ;
 ; RV64-LABEL: vec3_setcc_crash:
 ; RV64:       # %bb.0:
-; RV64-NEXT:    lwu a0, 0(a0)
+; RV64-NEXT:    lw a0, 0(a0)
 ; RV64-NEXT:    slli a2, a0, 40
 ; RV64-NEXT:    slli a3, a0, 56
 ; RV64-NEXT:    slli a4, a0, 48
@@ -73,7 +73,7 @@
 ; RV64-NEXT:    li a0, 0
 ; RV64-NEXT:    j .LBB0_8
 ; RV64-NEXT:  .LBB0_7:
-; RV64-NEXT:    srli a0, a0, 16
+; RV64-NEXT:    srliw a0, a0, 16
 ; RV64-NEXT:  .LBB0_8:
 ; RV64-NEXT:    sb a0, 2(a1)
 ; RV64-NEXT:    sh a2, 0(a1)
Index: llvm/test/CodeGen/RISCV/sextw-removal.ll
===================================================================
--- llvm/test/CodeGen/RISCV/sextw-removal.ll
+++ llvm/test/CodeGen/RISCV/sextw-removal.ll
@@ -65,9 +65,10 @@
 
 declare signext i32 @bar(i32 signext)
 
-; The load here will be an anyext load in isel and sext.w will be emitted for
-; the ret. Make sure we can look through logic ops to prove the sext.w is
-; unnecessary.
+; The load here was previously an aext load, but this has since been changed
+; to a signext load allowing us to remove a sext.w before isel. Thus we get
+; the same result with or without the sext.w removal pass.
+; Test has been left for coverage purposes.
 define signext i32 @test2(i32* %p, i32 signext %b) nounwind {
 ; RV64I-LABEL: test2:
 ; RV64I:       # %bb.0:
@@ -92,7 +93,6 @@
 ; NOREMOVAL-NEXT:    li a2, -2
 ; NOREMOVAL-NEXT:    rolw a1, a2, a1
 ; NOREMOVAL-NEXT:    and a0, a1, a0
-; NOREMOVAL-NEXT:    sext.w a0, a0
 ; NOREMOVAL-NEXT:    ret
   %a = load i32, i32* %p
   %shl = shl i32 1, %b
@@ -125,7 +125,6 @@
 ; NOREMOVAL-NEXT:    li a2, -2
 ; NOREMOVAL-NEXT:    rolw a1, a2, a1
 ; NOREMOVAL-NEXT:    or a0, a1, a0
-; NOREMOVAL-NEXT:    sext.w a0, a0
 ; NOREMOVAL-NEXT:    ret
   %a = load i32, i32* %p
   %shl = shl i32 1, %b
@@ -158,7 +157,6 @@
 ; NOREMOVAL-NEXT:    li a2, 1
 ; NOREMOVAL-NEXT:    sllw a1, a2, a1
 ; NOREMOVAL-NEXT:    xnor a0, a1, a0
-; NOREMOVAL-NEXT:    sext.w a0, a0
 ; NOREMOVAL-NEXT:    ret
   %a = load i32, i32* %p
   %shl = shl i32 1, %b
Index: llvm/lib/Target/RISCV/RISCVISelLowering.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -203,6 +203,8 @@
   if (Subtarget.is64Bit()) {
     setOperationAction(ISD::EH_DWARF_CFA, MVT::i64, Custom);
 
+    setOperationAction(ISD::LOAD, MVT::i32, Custom);
+
     setOperationAction({ISD::ADD, ISD::SUB, ISD::SHL, ISD::SRA, ISD::SRL},
                        MVT::i32, Custom);
 
@@ -6875,6 +6877,22 @@
     Results.push_back(RCW.getValue(2));
     break;
   }
+  case ISD::LOAD: {
+    if (!ISD::isNON_EXTLoad(N))
+      return;
+
+    // Use a SEXTLOAD instead of the default EXTLOAD. Similar to the
+    // sext_inreg we emit for ADD/SUB/MUL/SLLI.
+    LoadSDNode *Ld = cast<LoadSDNode>(N);
+
+    SDLoc dl(N);
+    SDValue Res = DAG.getExtLoad(ISD::SEXTLOAD, dl, MVT::i64, Ld->getChain(),
+                                 Ld->getBasePtr(), Ld->getMemoryVT(),
+                                 Ld->getMemOperand());
+    Results.push_back(DAG.getNode(ISD::TRUNCATE, dl, MVT::i32, Res));
+    Results.push_back(Res.getValue(1));
+    return;
+  }
   case ISD::MUL: {
     unsigned Size = N->getSimpleValueType(0).getSizeInBits();
     unsigned XLen = Subtarget.getXLen();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130397.446974.patch
Type: text/x-patch
Size: 3523 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220722/e1f089b6/attachment.bin>