[PATCH] D109729: [RISCV] Optimize (add (shl x, c0), (shl y, c1))

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D109729#2998987 <https://reviews.llvm.org/D109729#2998987>, @benshi001 wrote:

> In D109729#2998986 <https://reviews.llvm.org/D109729#2998986>, @craig.topper wrote:
>
>> In D109729#2998980 <https://reviews.llvm.org/D109729#2998980>, @benshi001 wrote:
>>
>>> Using `let PredicateCodeUsesOperands = 1` makes code more boring and complex. So I have resorted to using DAG2GAG selection which looks more clear.
>>
>> I'm not sure what you mean by boring here. Can you share one of the PatFrags using that feature?
>
> I mean the similar code is copied 6 times which is boring, and it becomes more complex than I expected. and I think using DAG2DAG is more clear.

The PredicateCodeUsesOperands feature should only require 3 PatFrags that look like this, but I can't get it to compile.

  let PredicateCodeUsesOperands = 1 in
  def AddShlShl_1A : PatFrag<(ops node:$A, node:$B, node:$C, node:$D),
                             (add (shl node:$A, node:$B),
                                  (shl node:$C, node:$D)), [{
    SDValue N0 = Operands[0], N1 = Operands[1];
    if (!N0.hasOneUse() || !N1.hasOneUse())
      return false;
    auto *N0C = cast<ConstantSDNode>(N0.getOperand(1));
    auto *N1C = cast<ConstantSDNode>(N1.getOperand(1));
    uint64_t C0 = N0C->getZExtValue(), C1 = N1C->getZExtValue();
    return C0 == C1 + 1;                                                                                                                                                       
  }]>;



================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:539
+
+    // Skip for vevtor types and larger types.
+    if (VT.isVector() || VT.getSizeInBits() > Subtarget->getXLen())
----------------
vevtor->vector


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:540
+    // Skip for vevtor types and larger types.
+    if (VT.isVector() || VT.getSizeInBits() > Subtarget->getXLen())
+      break;
----------------
The `VT.getSizeInBits() > Subtarget->getXLen()` should never fail. If the VT is scalar it must be XLenVT.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:584
+    // Build machine nodes.
+    SDLoc DL(Node);
+    SDValue NS = (C0 < C1) ? N0->getOperand(0) : N1->getOperand(0);
----------------
There's already a DL in the top of this function.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109729/new/

https://reviews.llvm.org/D109729