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