[llvm] [RISCV] Convert LWU to LW if possible in RISCVOptWInstrs (PR #144703)

[Alex Bradbury via llvm-commits]

asb wrote:

> Have you look at doing this during instruction selection using the hasAllNbitUsers in RISCVISelDAGToDAG.cpp

I'm limiting scope to just LWU => LW for now (there perhaps isn't much of an argument for LHU => LH beyond perhaps picking a "canonical form). I implemented this at ISel with the following patch:

```diff
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -208,7 +208,8 @@ bool RISCVInstructionSelector::hasAllNBitUsers(const MachineInstr &MI,
           MI.getOpcode() == TargetOpcode::G_AND ||
           MI.getOpcode() == TargetOpcode::G_OR ||
           MI.getOpcode() == TargetOpcode::G_XOR ||
-          MI.getOpcode() == TargetOpcode::G_SEXT_INREG || Depth != 0) &&
+          MI.getOpcode() == TargetOpcode::G_SEXT_INREG ||
+          MI.getOpcode() == TargetOpcode::G_ZEXTLOAD || Depth != 0) &&
          "Unexpected opcode");

   if (Depth >= RISCVInstructionSelector::MaxRecursionDepth)
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -3532,7 +3532,8 @@ bool RISCVDAGToDAGISel::hasAllNBitUsers(SDNode *Node, unsigned Bits,
           Node->getOpcode() == ISD::SRL || Node->getOpcode() == ISD::AND ||
           Node->getOpcode() == ISD::OR || Node->getOpcode() == ISD::XOR ||
           Node->getOpcode() == ISD::SIGN_EXTEND_INREG ||
-          isa<ConstantSDNode>(Node) || Depth != 0) &&
+          Node->getOpcode() == ISD::LOAD || isa<ConstantSDNode>(Node) ||
+          Depth != 0) &&
          "Unexpected opcode");

   if (Depth >= SelectionDAG::MaxRecursionDepth)
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -2083,6 +2083,13 @@ class binop_allwusers<SDPatternOperator operator>
   let GISelPredicateCode = [{ return hasAllWUsers(MI); }];
 }

+class unaryop_allwusers<SDPatternOperator operator>
+    : PatFrag<(ops node:$arg), (i64(operator node:$arg)), [{
+  return hasAllWUsers(N);
+}]> {
+  let GISelPredicateCode = [{ return hasAllWUsers(MI); }];
+}
+
 def sexti32_allwusers : PatFrag<(ops node:$src),
                                 (sext_inreg node:$src, i32), [{
   return hasAllWUsers(N);
@@ -2157,6 +2164,7 @@ def : Pat<(or_is_add 33signbits_node:$rs1, simm12:$imm),

 def : LdPat<sextloadi32, LW, i64>;
 def : LdPat<extloadi32, LW, i64>;
+def : LdPat<unaryop_allwusers<zextloadi32>, LW, i64>;
 def : LdPat<zextloadi32, LWU, i64>;
 def : LdPat<load, LD, i64>;
```

I've found that this is much less effective than the RISCVOptWInstrs change:
* Doing it at ISel results in ~4600 instructions changed across the test suite
* In RISCVOoptWInstrs (updated to do LWU=>LW only) changes ~20500 instructions
* There is no additional benefit in doing both (as you'd expected - but I ran this just to check).

I'm about to push changes to this patch to limit it to just the LWU change. Looking at the diffs between doing it at isel and this way, there are some cases where at first glance I would have thought would have been handled - I'll pick through a couple just to check there's nothing surprising going on.

https://github.com/llvm/llvm-project/pull/144703