[PATCH] D23445: [x86] Refactor a PowerPC specific ctlz/srl transformation (NFC).

[Sanjay Patel via llvm-commits]

spatel added a comment.

Just a couple of nits, otherwise LGTM. 
But please wait for Hal's feedback on the requested function comment.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3566-3567
@@ +3565,4 @@
+
+SDValue TargetLowering::LowerCmpEqZeroToCtlzSrl(SDValue Op,
+                                           SelectionDAG &DAG) const {
+  assert((Op->getOpcode() == ISD::SETCC) && "Input has to be a SETCC node.");
----------------
Indentation looks wrong - use clang-format.
Also, I know it's a giant mess, but I assume that we should use the current naming convention when creating a new function, so "Lower" -> "lower".

================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3572
@@ +3571,3 @@
+  ISD::CondCode CC = cast<CondCodeSDNode>(Op.getOperand(2))->get();
+  SDLoc dl(Op);
+  if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Op.getOperand(1))) {
----------------
I think it's ok for this patch to be a cut-and-paste and NFC, but a couple of possible improvements for a follow-up:
1. Sink 'dl' below the first two 'if' statements.
2. Do we need to use a target-specific type rather than hard-coding MVT::i32 under this?



https://reviews.llvm.org/D23445