<div dir="ltr">On 21 June 2013 18:40, Ye, Mei <span dir="ltr"><<a href="mailto:Mei.Ye@amd.com" target="_blank">Mei.Ye@amd.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">






<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="color:rgb(31,73,125);font-family:Calibri,sans-serif;font-size:11pt">What I take from this discussion is that optimization works should be pushed down to code gen whenever possible, which is fine with me. But it will be good
 to know that this is common guideline in this community.</span></p></div></div></blockquote><div><br></div><div style>Ok, lets ignore completely your patch, then, and start a new discussion: when good is good enough when it comes to benchmarks?</div>
<div style><br></div><div style>So far, I haven't seen any canonical way of benchmarking things, but I can give you some examples.</div><div style><br></div><div style>Big Changes</div><div style><br></div><div style>
Since December, the vectorizer has been worked from scratch to produce some amazing results. In the beginning, there were flags (-vectorize, -bb-vectorize, -slp-vectorize, etc) and they were not enabled by default under any optimization level. People started using, we turned on on our bots, tests and benchmarks and most people reported positive results with very little regressions.</div>
<div style><br></div><div style>Regressions were fixed and Nadav announced that he wanted to turn it on by default on -O3, just like GCC. People (including me) reported their findings, have done some more benchmarking and we (the community) concluded (via the mailing list) that it was, indeed, a good idea.</div>
<div style><br></div><div style>A few months later, and a lot more hard work from the guys, Nadav comes again and say: "Folks, we ran some benchmarks here, and I haven't found any significant regression of the vectorizer on -O2 and -Os, should we enable it by default?". That sparked even more controversy, folks (including me) run more benchmarks on their own platforms, programs, and problem sets, and the report was similar: the vectorizer rocks, even at -Os! Go for it!</div>
<div style><br></div><div style>And today (llvm 3.4svn, not 3.3), the vectorizer is enabled by default on -Os, -O2 and -O3.</div><div style><br></div><div style>So, it involved the large community, running very different benchmarks on very different hardware/software/problem-set configurations and only when everybody was happy, we made it default.</div>
<div style><br></div><div style>Your patch would jump over all that validation and there is no way you could run the benchmarks that we run because you don't have access to specialized hardware, or proprietary software or private data, that people in the community have, and care about. Even if you could buy all hardware on the planet, your evaluation (and mine too, for that matter), by itself, wouldn't be "good enough".</div>
<div style><br></div><div style>Small Changes</div><div style><br></div><div style>There are other ways in which the community does that, for instance, what to twinkle in the vectorizer. Once the vectorizer is done and enabled, engineers will run small benchmarks and will tweak parameters and sometimes that can hurt performance on private code. But, since the vectorizer is default already and those engineers are testing what they can, this then becomes a performance regression, a bug is filled and either fixed or reverted. The author + the affected parties will work together to fix it in private and upstream a better fix.</div>
<div style><br></div><div style>Why can't we do that with your patch, you may ask. Well, new features are more likely to affect a larger number of people than a small change on an already existing default feature, and there's no way you will be able to deal with all regression reports at the same time (imagine AMD fixing bugs on MIPS, ARM, SystemZ and Intel all at the same time because of a commit on a GPU optimization). In that case, what will happen is that your patch will be reverted and you'll have to start from scratch, or create an optional flag (that you should have done in the beginning).</div>
<div style><br></div><div style>When a compiler works on a single architecture and belongs to a single company, these kinds of instability are acceptable if the gain is big, and it seems to be in your case. But I learnt from experience, not many people care about 50% improvement in your own core as much as you do, and for them, the instability generated is at least a nuisance, and if it keeps repeating itself, people leave. We don't want that.</div>
<div style><br></div><div style>In a nutshell, there isn't a hammer in the world that will help you get your patch in. You'll need small, sharp tools and insert it with surgical precision. I suggest you follow up with Evan, he knows what he's talking about.</div>
<div style><br></div><div style>cheers,</div><div style>--renato</div></div></div></div>