[PATCH] D129606: [DAGCombine] fold (urem x, (lshr pow2, y)) -> (and x, (add (lshr pow2, y), -1))

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4596
     }
+    // TODO: We should sink the following into isKnowntoBePowerOfTwo
+    // using a OrZero parameter analogous to our handling in ValueTracking.
----------------
nit: Knownto -> KnownTo


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4607
+    if (N1.getOpcode() == ISD::SRL &&
+        DAG.isKnownToBeAPowerOfTwo(N1.getOperand(0))) {
+      // fold (urem x, (lshr pow2, y)) -> (and x, (add (lshr pow2, y), -1))
----------------
Is this code identical to the SHL code? Could we rewrite the if to 

```
   if ((N1.getOpcode() == ISD::SHL || N1.getOpcode() == ISD::SRL) &&
        DAG.isKnownToBeAPowerOfTwo(N1.getOperand(0))) {
```

And put the 
`// fold (urem x, (shl pow2, y)) -> (and x, (add (shl pow2, y), -1))` and `// fold (urem x, (lshr pow2, y)) -> (and x, (add (lshr pow2, y), -1))` comments above the if.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129606