[PATCH] D75114: [DAG] MatchRotate - Add funnel shift by immediate support

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.

LGTM - see inline for a nit.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6417-6418
+    SDValue Res;
+    if (IsRotate && (HasROTL || HasROTR))
+      Res = DAG.getNode(HasROTL ? ISD::ROTL : ISD::ROTR, DL, VT,
+                        LHSShiftArg, HasROTL ? LHSShiftAmt : RHSShiftAmt);
----------------
Nit: the code structure of these checks seems odd to me. 
If we're ok with using the "?:" to choose L/R for rotate, do the same for fsh?

    if (IsRotate && (HasROTL || HasROTR))
      Res = DAG.getNode(HasROTL ? ISD::ROTL : ISD::ROTR, DL, VT, LHSShiftArg,
                        HasROTL ? LHSShiftAmt : RHSShiftAmt);
    else
      Res = DAG.getNode(HasFSHL ? ISD::FSHL : ISD::FSHR, DL, VT, LHSShiftArg,
                        RHSShiftArg, HasFSHL ? LHSShiftAmt : RHSShiftAmt);



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75114