[PATCH] D41467: PR35710: Nary reassociation falls into infinite loop

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:466
                                                           BinaryOperator *I) {
+  // Need to avoid special case (C1*v)*(C2*v) = 0 because of overflow and
+  // there is Instruction "mul x, 0", as it leads to infinite loop
----------------
evstupac wrote:
> hfinkel wrote:
> > If I hadn't read the description, I'd have no idea what this meant (and I'm still not entirely sure). How does this detect overflow? Is it fair to say that x*0 is a special case because it can be  replicated arbitrarily-many times in products (because it is equal to 0, and thus, so is any product of which it is a part), and that can lead to infinite loops.
> The change is about to limit current implementation, which works fine with zeros. However, it does not expect to see 0 as a result of 2 non-zero expressions product (LHSExpr here).
> 
What doesn't expect to see a zero as the result of two non-zero expressions? And here we're also checking that RHS is foldable to zero? Which two expressions are non-zero?


Repository:
  rL LLVM

https://reviews.llvm.org/D41467