[PATCH] D155299: [AArch64][SVE2] Combine add+lsr to rshrnb for stores

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello. When adding these for NEON we were able to the transform via tablegen patterns at selection time, which has some benefits about leaving the nodes generic for as long as possible. In this case because of the differences in the instructions I would expect that it need demanded bits, so doing it as a combine probably makes sense.

It's best not to generate a MachineNode directly though if we can.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:20136
+  return SDValue(
+      DAG.getMachineNode(Opc, DL, VT,
+                         {Add->getOperand(0),
----------------
We should avoid creating machine nodes in SelectionDAG combines. It's like a layering violation. Can you change this to either generate an intrinsic, or preferably add a new AArch64ISD node for it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155299