[PATCH] D65366: [CodeGen][SelectionDAG] More efficient code for X % C == 0 (SREM case)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

It may have been a good idea not to post that particular diff after sleeping on it, not in sleep-deprived state :S



================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4796
 
+    // FIXME: in DAGCombine, we don't fold `rem %X, -C` to `rem %X, C`,
+    // so we may have D=-1 here, which will be handled differently from D=1.
----------------
craig.topper wrote:
> Would rem %X, -C -> rem %X, C even be valid for urem?
Uhm, no, i was going to drop the comment but forgot about it. Thanks.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5009
+    // Q = floor((2 * A) / (2^K))
+    APInt Q = (3 * A).udiv(APInt::getOneBitSet(W, K));
+
----------------
craig.topper wrote:
> Is there a trick here or is one of these a typo? The comment says 2*A and the code says 3*A.
This is a bug, i was trying to see if this would trigger any failures in test-suite (i think it didn't),
and forgot to tidy the diff before posting :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65366