[PATCH] D30350: [LSR] Add a cap for reassociation of AllFixupsOutsideLoop type LSRUse to protect compile time

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

I don't like a hanging test to be killed after timeout. I will try to create another testcase by checking the LSR trace.



================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:3429
+  // Arbitrarily choose a cap size of AddOps to protect compile time.
+  if (AddOps.size() >= 5 && LU.AllFixupsOutsideLoop)
+    return;
----------------
davide wrote:
> How did you chose this cap, BTW? 
> Maybe we should do some measurements and pick a less arbitrary value?
On one side, for AddOps.size()==5, even without the workaround, the compile time will not bloat up too much, so I think it can cover the cases suffering compile time issue. On the other side, not many cases have very large AddOps numbers. I choose 5 which is not a too small number so it will not kick in frequently. Since current model handling AllFixupsOutsideLoop is very imprecise,  I think it is better to just fix the compile time issue and don't change the existing code. 

I found no regressions using some internal benchmarks.




Repository:
  rL LLVM

https://reviews.llvm.org/D30350