[PATCH] D63391: [CodeGen] [SelectionDAG] More efficient code for X % C == 0 (UREM case) (try 2)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Logic looks right; I just pointed out some more formatting nits.

@RKSimon started reviewing the original patch, so let's see if he has comments.



================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:3398
 
+  // Fold remainder of division by a a constant
+  if (N0.getOpcode() == ISD::UREM && N0.hasOneUse() &&
----------------
"a a" 
Period at end of sentence.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:4400-4401
+/// Ref: "Hacker's Delight" 10-17.
+SDValue TargetLowering::buildUREMEqFold(EVT VT, SDValue REMNode,
+                                        SDValue CompNode, ISD::CondCode Cond,
+                                        DAGCombinerInfo &DCI,
----------------
Could shorten: 
REMNode -> Rem
CompNode -> check that this operand is zero before we call this, rather than pass it as a param?




================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:4430
+
+  // If MUL is unavailable, we cannot proceed in any case
+  if (!isOperationLegalOrCustom(ISD::MUL, REMVT))
----------------
Period at end of sentence.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:4434
+
+  // TODO: Add non-uniform constant support
+  ConstantSDNode *Divisor = isConstOrConstSplat(REMNode->getOperand(1));
----------------
Period at end of sentence.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:4454
+  // P = inv(D0, 2^W)
+  // 2^W requires W + 1 bits, so we have to extend and then truncate
+  unsigned W = D.getBitWidth();
----------------
Period at end of sentence.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:4471
+
+  // Rotate right only if D was even
+  if (DivisorIsEven) {
----------------
Period at end of sentence.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:4473
+  if (DivisorIsEven) {
+    // We need ROTR to do this
+    if (!isOperationLegalOrCustom(ISD::ROTR, REMVT))
----------------
Period at end of sentence.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63391