[PATCH] D54698: [SelectionDAG] Initial support for FSHL/FSHR funnel shift opcodes (PR39467)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7001
+  unsigned RotOpc = IsFSHL ? ISD::ROTL : ISD::ROTR;
+  if (N0 == N1 && hasOperation(RotOpc, VT))
+    return DAG.getNode(RotOpc, SDLoc(N), VT, N0, N2);
----------------
RKSimon wrote:
> nikic wrote:
> > Similar to what the DAG builder code does, would it make sense to also check for the reverse direction rotate here?
> Makes sense - always (and a SUB) or just for constant shift amounts?
Hard to say. If say FSHL is legal, but ROTL is not, but ROTR is, then it probably does not make sense to convert a legal FSHL to SUB, ROTR. Though that situation seems rather contrived.

Maybe it's best to wait with this transformation until it becomes necessary (if/when FSH is built directly.)


Repository:
  rL LLVM

https://reviews.llvm.org/D54698