[PATCH] D65047: [DAGCombine] matchBinOpReduction - add partial reduction matching

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:9055-9056
+  SDValue PrevOp;
   for (unsigned i = 0; i < Stages; ++i) {
+    unsigned MaskEnd = (1 << i);
+
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > This might warrant a comment/assertion that we do know that i is always less than 31.
> I indeed meant `31`, **not** `32`.
> Because if `i=31`, while there's no overflow, you get `(unsigned(1)<<31)`. which is `INT_MIN`,
> and then in 
> ```
> for (int Index = 0; Index < (int)MaskEnd; ++Index)
> ```
> you'll get at least a signed overflow, and maybe a deadlock.
> 
Err, other way around, the starting condition will be `0 s< INT_MIN`, and that is `false`,
so the loop won't run at all.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65047