[PATCH] D29862: LSR: an alternative way to resolve complex solution

[Chad Rosier via Phabricator via llvm-commits]

mcrosier added a comment.

In https://reviews.llvm.org/D29862#680367, @qcolombet wrote:

> LGTM with two caveats:
>
> 1. Should we set the option to true by default? I believe it would be best to keep it to false and send an email to llvm-dev to ask for benchmarking.


I strongly agree with Quentin in that this should have been committed with the feature disabled by default.  I appreciate that the regressions are being address, but that should happen prior to the feature being enabled.  I say this because this is a rather pervasive change that hasn't been full vetted on targets other than x86.

Here are some results from our LTO config on Kryo:
•	Spec2006/h264ref -2.8%
•	Spec2006/hmmer -8.9%
•	Spec2006/perlbench +3.5%
•	Spec2006/sjeng +2.3%
•	Spec2006/dealII +2.3%
•	Spec2006/sphinx3 -3.2%
•	Spec2000/crafty +2%
•	Spec2000/eon -2.4%
•	Spec2000/mcf -2.3%
•	Spec2000/perlbmk -2.8%
•	Spec2000/mesa -3.2%

As you can see this is a net loss.  Admittedly, this is a comparison against the prior week's performance and includes many changes, but we have at least bisected the the spec2006/hmmer regression to this commit (yes, I understand you have a fix pending, but that's beside the point).  I've asked Balaram @bmakam  to perform more exhaustive testing on our platform.  In the mean time, can you please disable this feature (as was originally suggested) until we've done some amount of due diligence?


Repository:
  rL LLVM

https://reviews.llvm.org/D29862