[PATCH] D36711: [X86] Combining CMOVs with [ANY, SIGN, ZERO]_EXTEND for cases where CMOV has constant arguments

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I checked in the tests from cmov-promotion.ll at https://reviews.llvm.org/rL311052, so we'd see the baseline codegen. Can you rebase after that?

I put a couple of style nit comments inline. I think this patch makes sense, but would appreciate if another reviewer can take a look too to make sure the non-cmov code is better too.



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:34359-34361
+static SDValue combineToExtendCMOV(SDValue N, EVT TargetVT,
+                                   unsigned ExtendOpcode, const SDLoc &dl,
+                                   SelectionDAG &DAG) {
----------------
It's a matter of taste, but seems more typical to shrink the function signature to just (SDNode *N, SelectionDAG &DAG) and then extract the VT, DL, and opcode in local variables.

Another style note: it's completely inconsistent, but I think we prefer "DL" now that variables are supposed to be capitalized.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:34379-34382
+    SmallVector<SDValue, 4> Ops = {CMovOp0, CMovOp1, N.getOperand(2),
+                                   N.getOperand(3)};
+
+    return DAG.getNode(X86ISD::CMOV, dl, TargetVT, Ops);
----------------
Could just use the 4 operand override of getNode here to avoid the explicit SmallVector?


https://reviews.llvm.org/D36711