Adding Freeze instruction and fix loop unswitch

Nuno Lopes via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 25 03:19:52 PST 2017


Quoting Friedman, Eli <efriedma at codeaurora.org>:

> 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?

Good point, thanks, I forgot.  I've added it here:  
https://reviews.llvm.org/D29121

Nuno



More information about the llvm-commits mailing list