Adding Freeze instruction and fix loop unswitch

Nuno Lopes via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 25 05:00:24 PST 2017


No, I don't think so.
We are not removing or changing the semantics of undef with this set  
of patches.
If we remove undef later, it seems we could use poison to replace  
undef in this case.

Nuno


Quoting David Majnemer <david.majnemer at gmail.com>:

> Will we need to change the shufflevector <
> http://llvm.org/docs/LangRef.html#shufflevector-instruction> semantics? It
> defines undef to be meaningful for the second vector.
>
> On Tue, Jan 24, 2017 at 10:37 AM, Friedman, Eli via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> On 1/23/2017 4:15 AM, Nuno Lopes via llvm-commits wrote:
>>
>>> Hi,
>>>
>>> Following our talk at the LLVM dev meeting regarding poison, we would
>>> like to introduce the freeze instruction in the LLVM IR so that we can fix
>>> a longstanding bug in loop unswitch.
>>> In the talk I mentioned a bigger proposal with several orthogonal
>>> changes. We are now splitting that proposal in smaller pieces, and this set
>>> of patches is the first piece.  These pieces are independent and can be
>>> reviewed independently.
>>> Juneyoung Lee implemented the whole thing; I'm just the messenger.
>>>
>>> A few details about the problem:
>>>
>>> Loop unswitch does the following transformation:
>>>   while (...) {
>>>     if (C) { A }
>>>     else   { B }
>>>   }
>>>
>>> into:
>>>   if (C) {
>>>     while (...) { A }
>>>   } else {
>>>     while (...) { B }
>>>   }
>>>
>>>
>>> This transformation is incorrect if C may be poison/undef, since
>>> branching on poison/undef is UB. If, for example, the loop never executed,
>>> the transformed code would be branching on C while the original was not,
>>> and so we introduced UB (if C is undef/poison).
>>>
>>> This has been shown to lead to end-to-end miscompilations when loop
>>> unswitch is combined with other passes, like GVN:
>>>  - https://llvm.org/bugs/show_bug.cgi?id=27506
>>>  - https://llvm.org/bugs/show_bug.cgi?id=31652
>>>
>>>
>>> We propose a simple change, which is to transform the code above into:
>>>   C' = freeze(c)
>>>   if (C') {
>>>     while (...) { A }
>>>   } else {
>>>     while (...) { B }
>>>   }
>>>
>>>
>>> Freeze always returns a non-poison/undef value. When given a poison/undef
>>> value, it returns a non-deterministic value that is fixed for all uses.
>>>
>>>
>>> We've split the changes in 5 patches:
>>>  - Add Freeze instruction to IR: https://reviews.llvm.org/D29011
>>>  - InstCombine/InstSimplify support for freeze:
>>> https://reviews.llvm.org/D29013
>>>  - SelectionDAG support for freeze: https://reviews.llvm.org/D29014
>>>  - Fix wrong code emission by loop unswitch:
>>> https://reviews.llvm.org/D29015
>>>  - Optimize placement of freeze in loop unswitch:
>>> https://reviews.llvm.org/D29016
>>>
>>
>> I'm not seeing any patch in this series with the necessary changes to
>> LangRef?
>>
>> -Eli
>>
>> --
>> Employee of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
>> Foundation Collaborative Project
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list