<div dir="ltr">My apologies, I meant to say:<div><span style="font-size:12.8px">To the best of my knowledge LVI is *now* consistent and preserved. I</span><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 29, 2017 at 12:13 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">Hello Philip,<div><br></div><div>While developing the preservation of DT across JT we were asked by Davide Italiano to see if we could also preserve LVI. When we did so we discovered a correctness bug in test <span style="font-size:12.8px">Transforms/</span><span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE" style="font-size:12.8px">JumpThreading</span><span style="font-size:12.8px">/</span><span style="font-size:12.8px"><wbr>pr33917.ll:</span></div><div><br></div><div><p class="MsoNormal" style="font-size:12.8px">FAIL: LLVM :: Transforms/<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">JumpThreading</span>/pr339<wbr>17.ll (19407 of 22252)<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">******************** TEST 'LLVM :: Transforms/<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">JumpThreading</span>/pr339<wbr>17.ll' FAILED ********************<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">Script:<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">--<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">/work/<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">brzycki</span>/upstream/build/x<wbr>86_e73dbb2/./bin/opt -jump-threading -correlated-propagation /work/brzycki/upstream/llvm-pr<wbr>oject/llvm/test/Transforms/Jum<wbr>pThreading/pr33917.ll -S | /work/<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">brzycki</span>/upstream/build/x<wbr>86_e73dbb2/./bin/<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">FileCheck</span>/wor<wbr>k/brzycki/upstream/llvm-projec<wbr>t/llvm/test/Transforms/JumpThr<wbr>eading/pr33917.ll<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">--<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">Exit Code: 1<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"><u></u> <u></u></p><p class="MsoNormal" style="font-size:12.8px">Command Output (<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">stderr</span>):<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">--<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">/work/brzycki/upstream/llvm-pr<wbr>oject/llvm/test/Transforms/Jum<wbr>pThreading/pr33917.ll:24:15: error: expected string not found in input<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">; CHECK-NEXT: [[T11:%.*]] = <span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">icmp</span> ne i8* [[T9]], null<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"> ^<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"><<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">stdin</span>>:23:7: note: scanning from here<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">good: ; <span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">preds</span> = %bb9<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"> ^<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"><<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">stdin</span>>:23:7: note: with variable "T9" equal to "%t9"<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">good: ; <span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">preds</span> = %bb9<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"> ^<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px"><<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">stdin</span>>:25:2: note: possible intended match here<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">%<span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">cond</span> = <span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">icmp</span> <span class="m_-6990191239697798196gmail-m_3891509515712620017gmail-m_-6131609275055286452SpellE">eq</span> i64 %t12, 1<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">^</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">Inspection of the .ll showed this optimization to be invalid. Further debug showed the culprit to be caused by LVI's conditional requirement of DT:</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.391923791] 0x25020a0 Executing Pass 'Function Pass Manager' on Module '/work/brzycki/upstream/llvm-p<wbr>roject/llvm/test/Transforms/Ju<wbr>mpThreading/pr33917.ll'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.392083604] 0x2514e00 Executing Pass 'Dominator Tree Construction' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.392201600] 0x2514e00 Executing Pass 'Basic Alias Analysis (stateless AA <span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">impl</span>)' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.392328316] 0x2514e00 Executing Pass 'Function Alias Analysis Results' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.392402761] 0x2514e00 Executing Pass 'Lazy Value Information Analysis' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.392431145] 0x2514e00 Executing Pass 'Jump Threading' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393124549] 0x2514e00 Freeing Pass 'Dominator Tree Construction' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393155519] 0x2514e00 Freeing Pass 'Jump Threading' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393173763] 0x2514e00 Freeing Pass 'Basic Alias Analysis (stateless AA <span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">impl</span>)' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393190916] 0x2514e00 Freeing Pass 'Function Alias Analysis Results' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393208870] 0x2514e00 Executing Pass 'Value Propagation' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393528791] 0x2514e00 Made Modification 'Value Propagation' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393566180] 0x2514e00 Freeing Pass 'Value Propagation' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393584428] 0x2514e00 Freeing Pass 'Lazy Value Information Analysis' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393618018] 0x2514e00 Executing Pass 'Module Verifier' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393809687] 0x2514e00 Freeing Pass 'Module Verifier' on Function '<span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">patatino</span>'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393833337] 0x25020a0 Made Modification 'Function Pass Manager' on Module '/work/brzycki/upstream/llvm-p<wbr>roject/llvm/test/Transforms/Ju<wbr>mpThreading/pr33917.ll'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393852785] 0x25020a0 Freeing Pass 'Assumption Cache Tracker' on Module '/work/brzycki/upstream/llvm-p<wbr>roject/llvm/test/Transforms/Ju<wbr>mpThreading/pr33917.ll'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393890918] 0x25020a0 Freeing Pass 'Target Library Information' on Module '/work/brzycki/upstream/llvm-p<wbr>roject/llvm/test/Transforms/Ju<wbr>mpThreading/pr33917.ll'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">[2017-09-29 11:51:08.393908905] 0x25020a0 Executing Pass 'Print Module IR' on Module '/work/brzycki/upstream/llvm-p<wbr>roject/llvm/test/Transforms/Ju<wbr>mpThreading/pr33917.ll'...<u></u><u></u></p><p class="MsoNormal" style="font-size:12.8px">; <span class="m_-6990191239697798196gmail-m_-6131609275055286452SpellE">ModuleID</span> = '/work/brzycki/upstream/llvm-p<wbr>roject/llvm/test/Transforms/Ju<wbr>mpThreading/pr33917.ll'</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">In this trace of the pass manager you can see the following situation:</p><p class="MsoNormal" style="font-size:12.8px">DT executed</p><p class="MsoNormal" style="font-size:12.8px">LVI executed (and using the previously constructed DT)</p><p class="MsoNormal" style="font-size:12.8px">JT executed</p><p class="MsoNormal" style="font-size:12.8px">Freeing of DT</p><p class="MsoNormal" style="font-size:12.8px">Freeing of JT</p><p class="MsoNormal" style="font-size:12.8px">Value Propagation executed</p><p class="MsoNormal" style="font-size:12.8px">Freeing of VP</p><p class="MsoNormal" style="font-size:12.8px">Freeing of LVI</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">The problem here is LVI was not re-generated before Value Propagation executed. This means that the pointer LVI had to DT is now garbage (it was freed after executing JT).</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">This is why we explicitly added the requirement for DT inside <wbr>CorrelatedValuePropagation.<wbr>cpp. This additional requirement informs the pass manager it cannot free DT after executing JT.</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">To the best of my knowledge LVI is not consistent and preserved. I am stating this cautiously because JT and VP are the only users of LVI today and they are consistent. What I do not know is if another pass were added that required LVI and not DT would the pass manager correctly execute/free the passes.</p><p class="MsoNormal" style="font-size:12.8px"><br></p><p class="MsoNormal" style="font-size:12.8px">In my opinion DT should be required by LVI to remove this potential landmine. When examining the history of the LVI pass I inferred from the commits that DT was made conditional due to the complexities of adding DT preservation to JT. With Jakub's simplified DT api and our work in using it I think it makes sense to enable LVI's requirement on DT. It would simplify the logic in the pass and prevent these transitive dependencies for the Pass manager to miss.</p></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 29, 2017 at 11:03 AM, Philip Reames 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">To be clear, you're saying that there was a bug, we worked around it by making jump-threading not preserve LVI/DT, fixed the bug, then re-enabled the preservation? If so, that's fine. It would have been very helpful if the submit comment mentioned that history.<div class="m_-6990191239697798196HOEnZb"><div class="m_-6990191239697798196h5"><br>
<br>
<br>
On 09/29/2017 08:49 AM, Sebastian Pop wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Fri, Sep 29, 2017 at 10:29 AM, Philip Reames via llvm-commits<br>
<<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Er, have you checked the history here? I thought this preservation was<br>
removed relatively recently to avoid a correctness bug.<br>
</blockquote>
I think the bug you mention was related to jump-threading not preserving LVI,<br>
and this is because LVI was computed by jump-threading<br>
and LVI relies on correct dominators.<br>
I believe that with the committed patch we addressed all the problems<br>
to preserve both dominators and LVI across jump-threading.<br>
Please let us know if you see other problems.<br>
<br>
Thanks,<br>
Sebastian<br>
</blockquote>
<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>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>