[PATCH] D68360: PR41162 Implement LKK remainder and divisibility algorithms [urem]

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3976
+
+  SDValue BuildUREM(SDNode *Node, SelectionDAG &DAG, bool IsAfterLegalization,
+                    SmallVectorImpl<SDNode *> &Created) const;
----------------
Add a BuildUREM specific comment and give BuildSDIVPow2 its comment back


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:3916-3917
 
+  if (DAG.isKnownNeverZero(N1) && !TLI.isIntDivCheap(VT, Attr)) {
+    if (isConstantOrConstantVector(N1)) {
+      // check if there is a div to combine with rem.
----------------
lebedev.ri wrote:
> ```
> if (!TLI.isIntDivCheap(VT, Attr) && isConstantOrConstantVector(N1) && DAG.isKnownNeverZero(N1)) {
> ```
+1 Avoid (more costly) known bits analysis by using an early out on cheaper checks.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4925
+
+  unsigned F = FVT.getScalarSizeInBits();
+
----------------
Can we call this something more descriptive than just 'F'?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4951
+
+    assert(!D.isNullValue() && "Divisor cannot be zero");
+
----------------
Maybe put the assert straight after the 'const APInt &D = DivisorConstant->getAPIntValue();' line?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4964
+
+  // numerator
+  SDValue Numerator = Node->getOperand(0);
----------------
superfluous comment?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4968
+
+  // divisor constant
+  SDValue Divisor = Node->getOperand(1);
----------------
superfluous comment?


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

https://reviews.llvm.org/D68360