<div dir="ltr">On Sat, Apr 6, 2013 at 2:01 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</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 class="HOEnZb"><div class="h5">----- Original Message -----<br>
> From: "Daniel Dunbar" <<a href="mailto:daniel@zuster.org">daniel@zuster.org</a>><br>
> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>, "Jakob Stoklund Olesen" <<a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a>>, "Commit Messages and Patches for LLVM"<br>

> <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>
> Sent: Saturday, April 6, 2013 2:40:41 PM<br>
> Subject: PATCH: Make TSVC benchmarks static data layout more predictable (PR14076)<br>
><br>
><br>
> Hi Hal,<br>
><br>
><br>
> As currently written, the performance of the TSVC benchmarks can<br>
> depend very heavily on the exact address assignment of the global<br>
> data arrays. Since that is something we do not (and are not likely<br>
> anytime soon) model in the compiler, this makes them suboptimal as<br>
> they are not testing what they purport to be.<br>
><br>
><br>
> The attached patch fixes this problem by making the static data<br>
> layout more predictable by moving all of the data arrays into a<br>
> single structure, so that the relative addresses are stable across<br>
> all platforms.<br>
><br>
><br>
> In particular, this resolves:<br>
> <a href="http://llvm.org/bugs/show_bug.cgi?id=14076" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=14076</a><br>
><br>
> because, at least on OS X but presumably other architectures, two of<br>
> the arrays in some of the benchmarks are very likely to end up at<br>
> exact 4K offsets from each other. This causes severe performance<br>
> degradation on some Intel platforms, in the worst case on<br>
> StatementReordering-flt this can double the runtime of the<br>
> benchmark. See:<br>
> <a href="http://software.intel.com/sites/products/documentation/doclib/stdxe/2013/amplifierxe/win/ug_docs/GUID-C801145A-A066-4C1A-B744-2B51AD89EFF6.htm" target="_blank">http://software.intel.com/sites/products/documentation/doclib/stdxe/2013/amplifierxe/win/ug_docs/GUID-C801145A-A066-4C1A-B744-2B51AD89EFF6.htm</a><br>

> for more information.<br>
><br>
><br>
> Even worse, on some Intel architectures (Sandy Bridge, at least),<br>
> when this problem is hit the runtime of the benchmark is no longer<br>
> predictable and can vary by up to 100% run-to-run (!!!).<br>
><br>
><br>
> I wrote the patch in such a way that I don't think it should impair<br>
> the compilers ability to perform any vectorization optimizations,<br>
> but wanted to run it past you before committing it.<br>
><br>
><br>
> What do you think?<br>
<br>
</div></div>First, thanks for working on this! I think using your patch will necessitate runtime overlap checks on any vectorization (because there is no way to otherwise determine without IPO than the pointers point to disjoint memory regions). This will also cut out a lot of BB-vectorization opportunities. Clang does not respect restrict on non-function parameters, right? In that case, we might need to pass the arrays to each function through restrict parameters.<br>
</blockquote><div><br></div><div style>Hmm, you are probably right. The patch was silly though, there is no need to runtime initialize the global array pointers. Attached a revised version which just statically initializes them to point into the global structure; AA should be able to look through them now and see they don't alias. I verified that AA did this on a trivial function, and I generated the IR with -O3 -fvectorize before and after and spot checked that structurally the same things are going on. Look ok?</div>
<div style><br></div><div style> - Daniel</div><div style> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
 -Hal<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
><br>
><br>
> I attached a snapshot from a before and after run with the patch<br>
> applied, showing the graphs from some of the benchmarks in the TSVC<br>
> suite that trigger this problem. Each run was done with 5 samples<br>
> each, and as you can see in the old version, the runtime of the<br>
> benchmarks is highly variable.<br>
><br>
><br>
> Thanks,<br>
> - Daniel<br>
> =<br>
</div></div></blockquote></div><br></div></div>