<div dir="ltr">Hey Gerolf,<div>You may want to read the followups where i explain exactly why the approach in this patch this is not equivalent to regular preservation.</div><div>I think pretty much all of your questions are answered.</div><div>The current patch's approach is equivalent to rebuilding the dominator tree O(number of changes jump threading makes to the CFG) times. Given that, it's actually *really* fast.</div><div>In fact, if you replace the applyUpdates calls with the equivalent dominator tree rebuilds, it is > 100x slower on most testcases i tried.</div><div>That said, it's probably not the approach we want, because, IMHO, it still can't be made fast enough.</div><div>I think we've pretty much reached consensus on that at this point.</div><div><br></div><div><br></div><div><div class="gmail_extra"><div class="gmail_quote">On Fri, Oct 13, 2017 at 3:03 PM, Gerolf Hoflehner <span dir="ltr"><<a href="mailto:ghoflehner@apple.com" target="_blank">ghoflehner@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><br></div><div><br></div>What would compile-time look like with post-pass (assuming that was an option) full updates rather than preservation? Where is the time spent when there is such an increase in the update? When you assume there are 100 passes (for simplicity of the argument) and you see an increase of 10% in one pass that points to non-linear behavior. A 1% increase overall would point to a 2X increase in one path, so a 10% increase points to 20X for one pass. From an extra dominator run I would expect an increase of much less than 1% in this scenario. Finally, consider that the CTMarks are relatively small benchmarks so would be afraid of eve higher costs to some bigger apps. <div><br></div><div>Also, I agree with Michael that for a cost we should have at least an idea about a measurable benefit.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-Gerolf</div></font></span><div><div class="h5"><div><br><div><blockquote type="cite"><div>On Oct 13, 2017, at 10:06 AM, Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com" target="_blank">mzolotukhin@apple.com</a>> wrote:</div><br class="m_-2777792156991486830Apple-interchange-newline"><div><div style="word-wrap:break-word">Hi,<div><br></div><div>My apologies, “to be fixed” is indeed a bad wording.</div><div><br></div><div>I do agree that preserving DT is generally good. However, what we get from it *currently* is only slowdowns with some gains expected *in future* when other passes are taught to preserve DT as well. I believe that when all relevant passes preserve DT we’ll see compile-time improvements, that’s why I don’t want to lose this work and propose to put it under a flag (also to avoid bitrot). But now we’re just doing extra work for nothing - SimplifyCFG scheduled right after JT destroys DT and we have to recompute it anyway.</div><div><br></div><div>This is the full list of regressions caused by this change detected by our bots:</div><div><br><b>O3 LTO</b>:</div><div>CTMark/7zip/7zip-benchmark:<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>2.2%</div><div>CTMark/ClamAV/clamscan:<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>3.2%</div><div>CTMark/sqlite3/sqlite3:<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>10.9%</div><div><br></div><div><b>O3 LTO -g</b>:</div><div>CTMark/7zip/7zip-benchmark<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>2.4%<br>CTMark/ClamAV/clamscan<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>3.3%<br>CTMark/sqlite3/sqlite3<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>8.9%</div><div><br></div><b>Os</b>:<br>CTMark/ClamAV/clamscan<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>1.4%<br>CTMark/sqlite3/sqlite3<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>3.2%<br><div><b><br></b></div><div><b>Os-g</b>:<br>CTMark/ClamAV/clamscan<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>1.3%<br>CTMark/sqlite3/sqlite3<span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span><span class="m_-2777792156991486830Apple-tab-span" style="white-space:pre-wrap"> </span>2.4%</div><div><br></div><div>Again, I’m not saying this change shouldn’t be in tree. I do want it in tree, but I prefer to avoid incurring any costs before we get any benefits.</div><div><br></div><div>Michael</div><div><br></div><div>PS: I mentioned only sqlite3 on O3 just because it was the easiest way to illustrate the slowdown.<br><br></div><div><br><div><blockquote type="cite"><div>On Oct 12, 2017, at 11:14 PM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>> wrote:</div><br class="m_-2777792156991486830Apple-interchange-newline"><div><div dir="ltr">Okay, I guess i'll jump in again.<div><br></div><div>I don't think there is anything here to be fixed on the DT algorithm side.</div><div>I think there is some confusion about what is going on here.</div><div>Here, it is preserving DT at just about all possible times in the pass (IE not just updating in the end).<br></div><div><br></div><div>That would normally take a fully dominator tree rebuild for each call you see to applyUpdates.</div><div><br></div><div>The 4% (or 10%) is the cost of keeping it correct at every point in the pass.</div><div>In that sense, 4% is really really good. You are saving probably 100's or 1000's of dominator tree rebuilds to get the equivalent preservation.</div><div><br></div><div>I suspect that 4% goes to nothing if instead of applying immediately, it batched the updates more.</div><div><br></div><div>This is being done presumably to enable some better optimization.</div><div><br></div><div>But i don't think there is anything to be fixed here.</div><div>You will not get that 4%/10% to 0% with the current "preserve everywhere at all times" approach being taken here.</div><div><br></div><div>You probably could get it to near zero by "batch all the updates, apply at end of jump threading", and probably even "batch all the updates, apply at end of each iteration", and maybe somewhere in between.</div><div><br></div><div>So if the desire really is to have it up to date at every possible program point in jump threading, 4/10% is the cost we'd have to accept.</div><div>No faster algorithms exist :)</div><div><br></div><div>The only other option is to batch more (which i believe should just work. You should be able to keep a more global update vector, and apply less, and it should just work)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 12, 2017 at 7:00 PM, Michael Zolotukhin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Oct 12, 2017, at 4:16 PM, Brian M. Rzycki <<a href="mailto:brzycki@gmail.com" target="_blank">brzycki@gmail.com</a>> wrote:</div><br class="m_-2777792156991486830m_5156249697022068101Apple-interchange-newline"><div><div dir="ltr">Hello Michael, thank you for the feedback regarding -flto. I'm afraid I'm still unable to reproduce your 10% regression.</div></div></blockquote></span>I’m pretty sure the 10% O3-LTO regression has the same nature as the 4% regression on just O3 which you’re seeing. Were you able to address it? I think if you fix it, the 10% regression will also disappear.</div><div><br></div><div>Thanks,</div><div>Michael<div><div class="m_-2777792156991486830h5"><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>First, I rebased both compilers to the following patch from this morning:</div><div> </div><div><font face="monospace, monospace">commit 6a7fd927949514fcea4db39be441ae<wbr>74b609e967</font></div><div><font face="monospace, monospace">Author: Alexey Bataev <<a href="mailto:a.bataev@hotmail.com" target="_blank">a.bataev@hotmail.com</a>></font></div><div><font face="monospace, monospace">Date: Thu Oct 12 15:18:41 2017 +0000</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"> [OPENMP] Fix PR34926: Fix handling of the array sections passed as</font></div><div><font face="monospace, monospace"> function params.</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"> Codegen could crash if the array section base expression is the</font></div><div><font face="monospace, monospace"> function parameter.</font></div><div> </div><div>In order to do full LTO I needed a main.c and a Makefile to link things</div><div>together. Here is my main.c and Makefile:</div><div><br></div><div><font face="monospace, monospace">$ cat main.c</font></div><div><font face="monospace, monospace">#include "sqlite3.h"</font></div><div><font face="monospace, monospace">int main(int argc, char** argv) {</font></div><div><font face="monospace, monospace"> sqlite3 *db;</font></div><div><font face="monospace, monospace"> char *zErrMsg = 0;</font></div><div><font face="monospace, monospace"> int rc;</font></div><div><font face="monospace, monospace"> rc = sqlite3_open("test.db", &db);</font></div><div><font face="monospace, monospace"> if (rc)</font></div><div><font face="monospace, monospace"> return 1;</font></div><div><font face="monospace, monospace"> sqlite3_close(db);</font></div><div><font face="monospace, monospace"> return 0;</font></div><div><font face="monospace, monospace">}</font></div><div><font face="monospace, monospace">$ cat Makefile</font></div><div><font face="monospace, monospace">PROGRAM := sqlite3_test</font></div><div><font face="monospace, monospace">OBJ := main.o sqlite3.o</font></div><div><font face="monospace, monospace">CFLAGS ?= -O3</font></div><div><font face="monospace, monospace">EXTRA_FLAGS := -DNDEBUG -DSTDC_HEADERS=1 -DSQLITE_OMIT_LOAD_EXTENSION=1</font></div><div><font face="monospace, monospace">EXTRA_FLAGS += -DSQLITE_THREADSAFE=0</font></div><div><font face="monospace, monospace">%.o: %.c</font></div><div><font face="monospace, monospace"> /usr/bin/time -p $(CC) $(CFLAGS) $(EXTRA_FLAGS) -c $< -o $@</font></div><div><font face="monospace, monospace">$(PROGRAM): $(OBJ)</font></div><div><font face="monospace, monospace"> /usr/bin/time -p $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJ)</font></div><div><font face="monospace, monospace">all: $(PROGRAM)</font></div><div><font face="monospace, monospace">clean:</font></div><div><font face="monospace, monospace"> @rm -f $(OBJ) $(PROGRAM)</font></div><div> </div><div>All these runs are on the same X86_64 processor I mentioned in my previous post. I performed 4 sets of runs: with and without -flto for each compiler.</div><div><div> </div><div><font face="monospace, monospace"><b># upstream with -O3 -flto, best of each makefile step, from 3 runs</b></font></div><div><font face="monospace, monospace">$ CFLAGS="-O3 -flto" LDFLAGS="-flto" \</font></div><div><font face="monospace, monospace"> CC="/work/brzycki/upstream/ins<wbr>tall/bin/clang" taskset -c 30 make -j 1</font></div><div><font face="monospace, monospace">main.c -o main.o</font></div><div><font face="monospace, monospace"> real 0.17</font></div><div><font face="monospace, monospace"> user 0.13</font></div><div><font face="monospace, monospace"> sys 0.02</font></div><div><font face="monospace, monospace">sqlite3.c -o sqlite3.o</font></div><div><font face="monospace, monospace"> real 383.58</font></div><div><font face="monospace, monospace"> user 383.37</font></div><div><font face="monospace, monospace"> sys 0.22</font></div><div><font face="monospace, monospace">-o sqlite3_test main.o sqlite3.o</font></div><div><font face="monospace, monospace"> real 33.74</font></div><div><font face="monospace, monospace"> user 33.69</font></div><div><font face="monospace, monospace"> sys 0.02</font></div><div><font face="monospace, monospace"><b>TOTAL REAL: 417.49</b></font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"><b># upstream with -O3, best of each makefile step, from 3 runs</b></font></div><div><font face="monospace, monospace">$ CFLAGS="-O3" LDFLAGS="" \</font></div><div><font face="monospace, monospace"> CC="/work/brzycki/upstream/ins<wbr>tall/bin/clang" taskset -c 30 make -j 1</font></div><div><font face="monospace, monospace">main.c -o main.o</font></div><div><font face="monospace, monospace"> real 0.18</font></div><div><font face="monospace, monospace"> user 0.15</font></div><div><font face="monospace, monospace"> sys 0.02</font></div><div><font face="monospace, monospace">sqlite3.c -o sqlite3.o</font></div><div><font face="monospace, monospace"> real 481.93</font></div><div><font face="monospace, monospace"> user 481.46</font></div><div><font face="monospace, monospace"> sys 0.40</font></div><div><font face="monospace, monospace">-o sqlite3_test main.o sqlite3.o</font></div><div><font face="monospace, monospace"> real 0.06</font></div><div><font face="monospace, monospace"> user 0.03</font></div><div><font face="monospace, monospace"> sys 0.00</font></div><div><font face="monospace, monospace"><b>TOTAL REAL: 482.17</b></font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"><b># dt with -O3 -flto, best of each makefile step, from 3 runs</b></font></div><div><font face="monospace, monospace">$ CFLAGS="-O3 -flto" LDFLAGS="-flto" \</font></div><div><font face="monospace, monospace"> CC="/work/brzycki/dt/install/b<wbr>in/clang" taskset -c 30 make -j 1</font></div><div><font face="monospace, monospace">main.c -o main.o</font></div><div><font face="monospace, monospace"> real 0.16</font></div><div><font face="monospace, monospace"> user 0.14</font></div><div><font face="monospace, monospace"> sys 0.01</font></div><div><font face="monospace, monospace">sqlite3.c -o sqlite3.o</font></div><div><font face="monospace, monospace"> real 401.57</font></div><div><font face="monospace, monospace"> user 401.08</font></div><div><font face="monospace, monospace"> sys 0.40</font></div><div><font face="monospace, monospace">-o sqlite3_test main.o sqlite3.o</font></div><div><font face="monospace, monospace"> real 33.83</font></div><div><font face="monospace, monospace"> user 33.72</font></div><div><font face="monospace, monospace"> sys 0.08</font></div><div><font face="monospace, monospace"><b>TOTAL REAL: 435.56</b></font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"><b># dt with -O3, best of each makefile step, from 3 runs</b></font></div><div><font face="monospace, monospace">$ CFLAGS="-O3" LDFLAGS="" \</font></div><div><font face="monospace, monospace"> CC="/work/brzycki/dt/install/b<wbr>in/clang" taskset -c 30 make -j 1</font></div><div><font face="monospace, monospace">main.c -o main.o</font></div><div><font face="monospace, monospace"> real 0.17</font></div><div><font face="monospace, monospace"> user 0.15</font></div><div><font face="monospace, monospace"> sys 0.01</font></div><div><font face="monospace, monospace">sqlite3.c -o sqlite3.o</font></div><div><font face="monospace, monospace"> real 492.56</font></div><div><font face="monospace, monospace"> user 491.84</font></div><div><font face="monospace, monospace"> sys 0.54</font></div><div><font face="monospace, monospace">-o sqlite3_test main.o sqlite3.o</font></div><div><font face="monospace, monospace"> real 0.05</font></div><div><font face="monospace, monospace"> user 0.03</font></div><div><font face="monospace, monospace"> sys 0.00</font></div></div><div><font face="monospace, monospace"><b>TOTAL REAL: 492.78</b></font><br></div><div><br></div><div>And the results:</div><div><br></div><div><div><font face="monospace, monospace">UPSTREAM</font></div><div><font face="monospace, monospace"> * O3 (482.17) vs O3 -flto (417.49) : 15.5% in favor of -flto</font></div><div><font face="monospace, monospace">DT</font></div><div><font face="monospace, monospace"> * O3 (492.78) vs O3 -flto (435.56) : 13.1% in favor if -flto</font></div><div><font face="monospace, monospace">COMPARATIVE</font></div><div><font face="monospace, monospace"> * O3 upstream (482.17) vs O3 dt (492.78) : 2.2% in favor of upstream</font></div><div><font face="monospace, monospace"> * O3 -flto upstream (417.49) vs O3 -flto dt (435.56) : 4.3% in favor of upstream</font></div></div><div><br></div><div>I did not have time to run these tests on Aarch64 and it's possible the difference only happens on that architecture. I'd appreciate any feedback on the testing if you think there are mistakes or areas to focus on. It's my hope that this code is accepted and is not placed under a flag: that would make it difficult for other passes to rely on the preservation of DT and LVI if they come after JT.</div><div><br></div><div>-Brian</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 12, 2017 at 12:16 PM, Michael Zolotukhin <span dir="ltr"><<a href="mailto:mzolotukhin@apple.com" target="_blank">mzolotukhin@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Oct 11, 2017, at 4:20 PM, Jakub (Kuba) Kuderski <<a href="mailto:kubakuderski@gmail.com" target="_blank">kubakuderski@gmail.com</a>> wrote:</div><br class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980Apple-interchange-newline"><div><div dir="ltr">Does fullLTO use a different pass manager configuration, or where does the difference come from?</div></div></blockquote></span>Yes, LTO uses a different pass pipeline. Please see lib/Transforms/IPO/PassManager<wbr>Builder.cpp for details. The difference might come from different inlining decisions, for example.</div><span class="m_-2777792156991486830m_5156249697022068101HOEnZb"><font color="#888888"><div><br></div><div>Michael</div></font></span><div><div class="m_-2777792156991486830m_5156249697022068101h5"><div><br><blockquote type="cite"><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 11, 2017 at 5:53 PM, Michael Zolotukhin <span dir="ltr"><<a href="mailto:mzolotukhin@apple.com" target="_blank">mzolotukhin@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Oct 11, 2017, at 2:40 PM, Brian M. Rzycki <<a href="mailto:brzycki@gmail.com" target="_blank">brzycki@gmail.com</a>> wrote:</div><br class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002Apple-interchange-newline"><div><div dir="ltr">I did more testing on the DT/LVI patch for JumpThreading and I am unable to reproduce the 10% sqlite3 regression Michael Zolotukhin reported.</div></div></blockquote></span>Could you please try building full sqlite3 test with -flto? The 10% regression was detected on an O3-LTO bot, while in my example (just O3) I also saw just 5%. That’s what you’re probably seeing now (plus-minus differences in our configurations and noise).</div><div><br></div><div>Thanks for the testing!</div><span class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980HOEnZb"><font color="#888888"><div><br></div></font></span><div><span class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980HOEnZb"><font color="#888888">Michael</font></span><div><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980h5"><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div><div>There are two compilers: upstream and dt. The upstream compiler has the</div><div>following patch as its HEAD:</div><div><font face="monospace, monospace">$ git log HEAD</font></div><div><font face="monospace, monospace">commit c491ccc7bbddf8c04885fd3ab90f90<wbr>cc9c319d33 (HEAD -> master)</font></div><div><font face="monospace, monospace">Author: Ayal Zaks <<a href="mailto:ayal.zaks@intel.com" target="_blank">ayal.zaks@intel.com</a>></font></div><div><font face="monospace, monospace">Date: Thu Oct 5 15:45:14 2017 +0000</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> [LV] Fix PR34743 - handle casts that sink after interleaved loads</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> When ignoring a load that participates in an interleaved group, make</font></div><div><font face="monospace, monospace"> sure to move a cast that needs to sink after it.</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> Testcase derived from reproducer of PR34743.</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> Differential Revision: <a href="https://reviews.llvm.org/D38338" target="_blank">https://reviews.llvm.org/D3833<wbr>8</a></font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">The dt compiler is based on the exact same patch as upstream, only with</font></div><div><font face="monospace, monospace"><a href="https://reviews.llvm.org/D38558" target="_blank">https://reviews.llvm.org/D3855<wbr>8</a> applied.</font></div><div><br></div><div>The sqlite version:</div><div><font face="monospace, monospace">$ cat sqlite/VERSION</font></div><div><font face="monospace, monospace">3.20.1</font></div><div><br></div><div>The host is an X86 server-class CPU:</div><div><font face="monospace, monospace">processor : 30</font></div><div><font face="monospace, monospace">vendor_id : GenuineIntel</font></div><div><font face="monospace, monospace">cpu family : 6</font></div><div><font face="monospace, monospace">model : 79</font></div><div><font face="monospace, monospace">model name : Intel(R) Xeon(R) CPU E5-2667 v4 @ 3.20GHz</font></div><div><font face="monospace, monospace">stepping : 1</font></div><div><br></div><div>Here are the compiler runs:</div><div><br></div><div><font face="monospace, monospace"># upstream compiler, best of 3 runs</font></div><div><font face="monospace, monospace">$ time taskset -c 30 /work/brzycki/upstream/install<wbr>/bin/clang -DNDEBUG -O3 -DSTDC_HEADERS=1 -DSQLITE_OMIT_LOAD_EXTENSION=1 -DSQLITE_THREADSAFE=0 -o sqlite3.c.o -c sqlite3.c</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace">real 7m57.157s</font></div><div><font face="monospace, monospace">user 7m56.472s</font></div><div><font face="monospace, monospace">sys 0m0.560s</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace"># dt patched compiler, best of 3 runs</font></div><div><font face="monospace, monospace">$ time taskset -c 30 /work/brzycki/dt/install/bin/c<wbr>lang -DNDEBUG -O3 -DSTDC_HEADERS=1 -DSQLITE_OMIT_LOAD_EXTENSION=1 -DSQLITE_THREADSAFE=0 -o sqlite3.c.o -c sqlite3.c</font></div><div><font face="monospace, monospace"> </font></div><div><font face="monospace, monospace">real 8m14.753s</font></div><div><font face="monospace, monospace">user 8m13.932s</font></div><div><font face="monospace, monospace">sys 0m0.496s</font></div><div> </div><div>That’s a difference of 4.3%. I'm unsure why Arm64 is showing a further regression of 6%. I've also done some native Aarch64 Linux testing on a Firefly ARM72 device and see a difference very close to this number. If anyone is interested I can post this data too.</div></div><div><br></div><div>I plan on re-running these tests when <a href="https://reviews.llvm.org/D38802" target="_blank">https://reviews.llvm.org/<wbr>D38802</a> lands to see if it further reduces the overhead or not.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 4, 2017 at 3:12 PM, Brian M. Rzycki <span dir="ltr"><<a href="mailto:brzycki@gmail.com" target="_blank">brzycki@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I have just pushed two patches into Phabricator:<div>Take two of the DT/LVI patch:</div><div><p class="MsoNormal" style="font-size:12.8px"><a href="https://reviews.llvm.org/D38558" target="_blank">https://reviews.llvm.org/D3855<wbr>8</a><u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">A patch to place DT/LVI preservation under a flag:</p><p class="MsoNormal" style="font-size:12.8px"><a href="https://reviews.llvm.org/D38559" target="_blank">https://reviews.llvm.org/D3855<wbr>9</a></p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">Jakub, I too am planning on doing some profiling later this week. I would like to collaborate to minimize duplicated efforts.</p></div></div><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002HOEnZb"><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 4, 2017 at 12:01 PM, Jakub (Kuba) Kuderski via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I will try to profile the sqlite compilation later this week. It is not easy to come up with some benchmarks for the batch updater, and because of that it hasn't really been optimized yet. JumpThreading seems to be the first pass to put significant pressure on the incremental updater, so it'd be valuable to learn how much faster we can get there.</div><div class="gmail_extra"><div><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790h5"><br><div class="gmail_quote">On Wed, Oct 4, 2017 at 12:47 PM, Michael Zolotukhin <span dir="ltr"><<a href="mailto:mzolotukhin@apple.com" target="_blank">mzolotukhin@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Oct 4, 2017, at 9:25 AM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>> wrote:</div><br class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967Apple-interchange-newline"><div><div dir="ltr">+jakub for real.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 4, 2017 at 9:21 AM, Daniel Berlin <span dir="ltr"><<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">+jakub<div><br></div><div>I don't think preservation should be controlled by a flag. That seems like not a great path forward.</div><div>Either we should make it fast enough, or we should give up :)</div><div><br></div><div>Do you have any profiles on this?</div></div></blockquote></div></div></div></blockquote></span>I don’t, but it seems pretty clear what’s going on there - we do an extra work (preserving DT) for no benefits (we’re recomputing it a couple of passes later anyway). The pass after which it’s recomputed is SimplifyCFG, and I think it would be a much bigger endeavor to teach it to account for DT - so putting it under a flag for JumpThreading could help doing this work incrementally.</div><span class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036HOEnZb"><font color="#888888"><div><br></div><div>Michael</div></font></span><div><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036h5"><div><br><blockquote type="cite"><div><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 dir="ltr"><div><br><br></div></div><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967HOEnZb"><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967h5"><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Sep 30, 2017 at 12:19 PM, Michael Zolotukhin via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">For the record, we also detected significant compile-time increases caused by this patch. It caused up to 10% slow downs on sqlite3 and some other tests from CTMark/LLVM testsuite (on O3/LTO and other optsets).<div><br></div><div>Reproducer:</div><div><br></div><div>time clang-r314435/bin/clang -DNDEBUG -O3 -arch arm64 -isysroot $SYSROOT -DSTDC_HEADERS=1 -DSQLITE_OMIT_LOAD_EXTENSION=<wbr>1 -DSQLITE_THREADSAFE=0 -w -o sqlite3.c.o -c $TESTSUITE/CTMark/sqlite3/sqli<wbr>te3.c</div><div><div>real 0m20.235s</div><div>user 0m20.130s</div><div>sys 0m0.091s</div></div><div><br></div><div><div>time clang-r314432/bin/clang -DNDEBUG -O3 -arch arm64 -isysroot $SYSROOT -DSTDC_HEADERS=1 -DSQLITE_OMIT_LOAD_EXTENSION=<wbr>1 -DSQLITE_THREADSAFE=0 -w -o sqlite3.c.o -c $TESTSUITE/CTMark/sqlite3/sqli<wbr>te3.c</div><div><div>real 0m19.254s</div><div>user 0m19.154s</div><div>sys 0m0.094s</div></div><div><br></div><div>JumpThreading started to consume much longer time trying to preserve DT, but DT was still being recomputed couple of passes later. In general, I agree that we should preserve DT and in the end it should be beneficial for compile time too, but can we put it under a flag for now until we can actually benefit from it?</div><div><br></div><div>Thanks,</div><div>Michael</div><div></div></div><div><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529h5"><div><br></div><div><br></div><div><div><blockquote type="cite"><div>On Sep 30, 2017, at 5:02 AM, Daniel Jasper via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239Apple-interchange-newline"><div><div dir="ltr">I found other places where this Clang was running into this segfault. Reverted for now in r314589.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 29, 2017 at 10:19 PM, Friedman, Eli via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><span>
<div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-cite-prefix">On 9/28/2017 10:24 AM, Evandro Menezes
via llvm-commits wrote:<br>
</div>
<blockquote type="cite">
<pre>Author: evandro
Date: Thu Sep 28 10:24:40 2017
New Revision: 314435
URL: <a class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project?rev=314435&view=rev" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=314435&view=rev</a>
Log:
[JumpThreading] Preserve DT and LVI across the pass
JumpThreading now preserves dominance and lazy value information across the
entire pass. The pass manager is also informed of this preservation with
the goal of DT and LVI being recalculated fewer times overall during
compilation.
This change prepares JumpThreading for enhanced opportunities; particularly
those across loop boundaries.
Patch by: Brian Rzycki <a class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-txt-link-rfc2396E" href="mailto:b.rzycki@samsung.com" target="_blank"><b.rzycki@samsung.com></a>,
Sebastian Pop <a class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-txt-link-rfc2396E" href="mailto:s.pop@samsung.com" target="_blank"><s.pop@samsung.com></a>
Differential revision: <a class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-txt-link-freetext" href="https://reviews.llvm.org/D37528" target="_blank">https://reviews.llvm.org/D3752<wbr>8</a>
</pre>
</blockquote>
<br></span>
It looks like this is causing an assertion failure on the polly aosp
buildbot
(<a class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-txt-link-freetext" href="http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable/builds/266/steps/build-aosp/logs/stdio" target="_blank">http://lab.llvm.org:8011/buil<wbr>ders/aosp-O3-polly-before-vect<wbr>orizer-unprofitable/builds/266<wbr>/steps/build-aosp/logs/stdio</a>):<br>
<pre style="font-family:'Courier New',courier,monotype,monospace;font-size:inherit;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;word-spacing:0px"><span class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947stdout" style="font-family:'Courier New',courier,monotype,monospace">clang: /var/lib/buildbot/slaves/hexag<wbr>on-build-03/aosp/llvm.src/incl<wbr>ude/llvm/Support/GenericDomTre<wbr>eConstruction.h:906: static void llvm::DomTreeBuilder::SemiNCAI<wbr>nfo<llvm::DominatorTreeBase<ll<wbr>vm::BasicBlock, false> >::DeleteEdge(DomTreeT &, const BatchUpdatePtr, const NodePtr, const NodePtr) [DomTreeT = llvm::DominatorTreeBase<llvm::<wbr>BasicBlock, false>]: Assertion `!IsSuccessor(To, From) && "Deleted edge still exists in the CFG!"' failed.
#0 0x00000000014f6fc4 PrintStackTraceSignalHandler(v<wbr>oid*) (llvm.inst/bin/clang+0x14f6fc4<wbr>)
#1 0x00000000014f72d6 SignalHandler(int) (llvm.inst/bin/clang+0x14f72d6<wbr>)
#2 0x00007f694afddd10 __restore_rt (/lib/x86_64-linux-gnu/libpthr<wbr>ead.so.0+0x10d10)
#3 0x00007f6949bbb267 gsignal /build/buildd/glibc-2.21/signa<wbr>l/../sysdeps/unix/sysv/linux/r<wbr>aise.c:55:0
#4 0x00007f6949bbceca abort /build/buildd/glibc-2.21/stdli<wbr>b/abort.c:91:0
#5 0x00007f6949bb403d __assert_fail_base /build/buildd/glibc-2.21/asser<wbr>t/assert.c:92:0
#6 0x00007f6949bb40f2 (/lib/x86_64-linux-gnu/libc.so<wbr>.6+0x2e0f2)
#7 0x000000000104a1dc (llvm.inst/bin/clang+0x104a1dc<wbr>)
#8 0x0000000001357fd9 llvm::JumpThreadingPass::Proce<wbr>ssBlock(llvm::BasicBlock*) (llvm.inst/bin/clang+0x1357fd9<wbr>)
#9 0x0000000001356e6e llvm::JumpThreadingPass::runIm<wbr>pl(llvm::Function&, llvm::TargetLibraryInfo*, llvm::LazyValueInfo*, llvm::AAResults*, llvm::DominatorTree*, bool, std::unique_ptr<llvm::BlockFre<wbr>quencyInfo, std::default_delete<llvm::Bloc<wbr>kFrequencyInfo> >, std::unique_ptr<llvm::BranchPr<wbr>obabilityInfo, std::default_delete<llvm::Bran<wbr>chProbabilityInfo> >) (llvm.inst/bin/clang+0x1356e6e<wbr>)
#10 0x0000000001363c49 (anonymous namespace)::JumpThreading::run<wbr>OnFunction(llvm::Function&) (llvm.inst/bin/clang+0x1363c49<wbr>)
#11 0x00000000010a53c4 llvm::FPPassManager::runOnFunc<wbr>tion(llvm::Function&) (llvm.inst/bin/clang+0x10a53c4<wbr>)
#12 0x00000000031f7376 (anonymous namespace)::CGPassManager::run<wbr>OnModule(llvm::Module&) (llvm.inst/bin/clang+0x31f7376<wbr>)
#13 0x00000000010a5b05 llvm::legacy::PassManagerImpl:<wbr>:run(llvm::Module&) (llvm.inst/bin/clang+0x10a5b05<wbr>)
#14 0x0000000001688beb clang::EmitBackendOutput(clang<wbr>::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwri<wbr>te_stream, std::default_delete<llvm::raw_<wbr>pwrite_stream> >) (llvm.inst/bin/clang+0x1688beb<wbr>)
#15 0x0000000001ed7672 clang::BackendConsumer::Handle<wbr>TranslationUnit(clang::ASTCont<wbr>ext&) (llvm.inst/bin/clang+0x1ed7672<wbr>)
#16 0x0000000002195235 clang::ParseAST(clang::Sema&, bool, bool) (llvm.inst/bin/clang+0x2195235<wbr>)
#17 0x0000000001a75628 clang::FrontendAction::Execute<wbr>() (llvm.inst/bin/clang+0x1a75628<wbr>)
#18 0x0000000001a37181 clang::CompilerInstance::Execu<wbr>teAction(clang::FrontendAction<wbr>&) (llvm.inst/bin/clang+0x1a37181<wbr>)
#19 0x0000000001b06501 clang::ExecuteCompilerInvocati<wbr>on(clang::CompilerInstance*) (llvm.inst/bin/clang+0x1b06501<wbr>)
#20 0x000000000080e433 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (llvm.inst/bin/clang+0x80e433)
#21 0x000000000080c997 main (llvm.inst/bin/clang+0x80c997)
#22 0x00007f6949ba6a40 __libc_start_main /build/buildd/glibc-2.21/csu/l<wbr>ibc-start.c:323:0
#23 0x0000000000809479 _start (llvm.inst/bin/clang+0x809479)
</span></pre>
I'll try to come up with a reduced testcase this afternoon.<br class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947Apple-interchange-newline">
<br>
-Eli<span class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239HOEnZb"><font color="#888888"><br>
<pre class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036m_4776887672213244967m_-1877988520402087529m_5149017397612556239m_-3717849707277701947moz-signature" cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
</font></span></div>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
______________________________<wbr>_________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></div></div></div><br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></blockquote></div><br></div></div></div></blockquote></div><br><br clear="all"><div><br></div></div></div><span class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790HOEnZb"><font color="#888888">-- <br><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980m_-8952630210461987002m_-329897501231553790m_-1544835902761221036gmail_signature" data-smartmail="gmail_signature"><div>Jakub Kuderski</div></div>
</font></span></div>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></blockquote></div></div></div><br></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="m_-2777792156991486830m_5156249697022068101m_-8291752217458721980gmail_signature" data-smartmail="gmail_signature"><div>Jakub Kuderski</div></div>
</div>
</div></blockquote></div><br></div></div></div></blockquote></div><br></div>
</div></blockquote></div></div></div><br></div><br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></blockquote></div><br></div></div></div></blockquote></div><br></div></div></div></div></blockquote></div><br></div></div></div>