[PATCH] D30350: [LSR] Add a cap for reassociation of AllFixupsOutsideLoop type LSRUse to protect compile time
Wei Mi via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 27 09:53:38 PST 2017
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
More information about the llvm-commits
mailing list