[llvm] r309355 - [JumpThreading] Stop falsely preserving LazyValueInfo.

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 28 14:38:13 PDT 2017


On Fri, Jul 28, 2017 at 2:12 PM, Davide Italiano <davide at freebsd.org> wrote:
> On Fri, Jul 28, 2017 at 2:05 PM, Hans Wennborg <hans at chromium.org> wrote:
>> On Thu, Jul 27, 2017 at 8:12 PM, Davide Italiano <davide at freebsd.org> wrote:
>>> On Thu, Jul 27, 2017 at 8:10 PM, Davide Italiano via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>> Author: davide
>>>> Date: Thu Jul 27 20:10:43 2017
>>>> New Revision: 309355
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=309355&view=rev
>>>> Log:
>>>> [JumpThreading] Stop falsely preserving LazyValueInfo.
>>>>
>>>> JumpThreading claims to preserve LVI, but it doesn't preserve
>>>> the analyses which LVI holds a reference to (e.g. the Dominator).
>>>> In the current pass manager infrastructure, after JT runs, the
>>>> PM frees these analyses (including DominatorTree) but preserves
>>>> LVI.
>>>>
>>>> CorrelatedValuePropagation runs immediately after and queries
>>>> a corrupted domtree, causing weird miscompiles.
>>>>
>>>> This commit disables the preservation of LVI for the time being.
>>>> Eventually, we should either move LVI to a proper dependency
>>>> tracking mechanism (i.e. an analyses shouldn't hold references
>>>> to other analyses and compute them on demand if needed), or
>>>> we should teach all the passes preserving LVI to preserve the
>>>> analyses LVI depends on.
>>>>
>>>> The new pass manager has a mechanism to invalidate LVI in case
>>>> one of the analyses it depends on becomes invalid, so this problem
>>>> shouldn't exist (at least not in this immediate form), but handling
>>>> of analyses holding references is still a very delicate subject.
>>>>
>>>> Fixes PR33917 (and rustc).
>>>>
>>>
>>> Hans, we might consider backporting this (which also depends on r309353) to 5.0.
>>
>> How does it depend on r309353? It looks to me like that just adds a
>> debugging option?
>
> We don't preserve LVI anymore so when the printer runs, the cache is
> empty and the test lvi-after-jumpthreading.ll fails.
> The alternative to not backport r309353 is that of removing the test altogether.

Thanks for explaining. I've merged both in r309439.

Thanks,
Hans


More information about the llvm-commits mailing list