[llvm] r219462 - SimplifyCFG: Don't convert phis into selects if we could remove undef behavior

[Arnold Schwaighofer]

Right, however the code is already there and also handles the more general:

  define void @test6(i1 %cond, i8* %ptr) {
  entry:
    br i1 %cond, label %bb1, label %bb2

  bb1:
    call void foobar()
    br label %bb2

  bb2:
    %ptr.2 = phi i8* [ %ptr, %entry ], [ null, %bb1 ]
    store i8 2, i8* %ptr.2, align 8
    ret void
  }

=> 

  define void @test6(i1 %cond, i8* %ptr) {
  entry:
    br i1 %cond, label %bb1, label %bb2

  bb1:
    call void foobar()
    unreachable

  bb2:
    store i8 2, i8* %ptr, align 8
    ret void
  }

There is an argument to be made to do both I guess ...

> On Oct 10, 2014, at 11:53 AM, Philip Reames <listmail at philipreames.com> wrote:
> 
> Why is this pattern any worse for recognizing undefined behaviour?
> 
>  define void @test6(i1 %cond, i8* %ptr) {
>    %ptr.2 = select i1 %cond, i8* null, i8* %ptr
>    store i8 2, i8* %ptr.2, align 8
>    ret void
>  }
> 
> We can infer that %cond can't be true, or undefined behaviour would result.  This might not be implemented currently, but it certainly seems easy to do.
> 
> That transformation would seem to get us the same result:
> 
>  define void @test6(i1 %cond, i8* %ptr) {
>    store i8 2, i8* %ptr.2, align 8
>    ret void
>  }
> 
> Philip
> 
> On 10/09/2014 06:27 PM, Arnold Schwaighofer wrote:
>> Author: arnolds
>> Date: Thu Oct  9 20:27:02 2014
>> New Revision: 219462
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=219462&view=rev
>> Log:
>> SimplifyCFG: Don't convert phis into selects if we could remove undef behavior
>> instead
>> 
>> We used to transform this:
>> 
>>   define void @test6(i1 %cond, i8* %ptr) {
>>   entry:
>>     br i1 %cond, label %bb1, label %bb2
>> 
>>   bb1:
>>     br label %bb2
>> 
>>   bb2:
>>     %ptr.2 = phi i8* [ %ptr, %entry ], [ null, %bb1 ]
>>     store i8 2, i8* %ptr.2, align 8
>>     ret void
>>   }
>> 
>> into this:
>> 
>>   define void @test6(i1 %cond, i8* %ptr) {
>>     %ptr.2 = select i1 %cond, i8* null, i8* %ptr
>>     store i8 2, i8* %ptr.2, align 8
>>     ret void
>>   }
>> 
>> because the simplifycfg transformation into selects would happen to happen
>> before the simplifycfg transformation that removes unreachable control flow
>> (We have 'unreachable control flow' due to the store to null which is undefined
>> behavior).
>> 
>> The existing transformation that removes unreachable control flow in simplifycfg
>> is:
>> 
>>   /// If BB has an incoming value that will always trigger undefined behavior
>>   /// (eg. null pointer dereference), remove the branch leading here.
>>   static bool removeUndefIntroducingPredecessor(BasicBlock *BB)
>> 
>> Now we generate:
>> 
>>   define void @test6(i1 %cond, i8* %ptr) {
>>     store i8 2, i8* %ptr.2, align 8
>>     ret void
>>   }
>> 
>> I did not see any impact on the test-suite + externals.
>> 
>> rdar://18596215
>> 
>> Modified:
>>     llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>>     llvm/trunk/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
>> 
>> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=219462&r1=219461&r2=219462&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Oct  9 20:27:02 2014
>> @@ -1010,6 +1010,8 @@ static bool isSafeToHoistInvoke(BasicBlo
>>    return true;
>>  }
>>  +static bool passingValueIsAlwaysUndefined(Value *V, Instruction *I);
>> +
>>  /// HoistThenElseCodeToIf - Given a conditional branch that goes to BB1 and
>>  /// BB2, hoist any common code in the two blocks up into the branch block.  The
>>  /// caller of this function guarantees that BI's block dominates BB1 and BB2.
>> @@ -1094,6 +1096,12 @@ HoistTerminator:
>>        if (BB1V == BB2V)
>>          continue;
>>  +      // Check for passingValueIsAlwaysUndefined here because we would rather
>> +      // eliminate undefined control flow then converting it to a select.
>> +      if (passingValueIsAlwaysUndefined(BB1V, PN) ||
>> +          passingValueIsAlwaysUndefined(BB2V, PN))
>> +       return Changed;
>> +
>>        if (isa<ConstantExpr>(BB1V) && !isSafeToSpeculativelyExecute(BB1V, DL))
>>          return Changed;
>>        if (isa<ConstantExpr>(BB2V) && !isSafeToSpeculativelyExecute(BB2V, DL))
>> @@ -1508,6 +1516,11 @@ static bool SpeculativelyExecuteBB(Branc
>>      if (ThenV == OrigV)
>>        continue;
>>  +    // Don't convert to selects if we could remove undefined behavior instead.
>> +    if (passingValueIsAlwaysUndefined(OrigV, PN) ||
>> +        passingValueIsAlwaysUndefined(ThenV, PN))
>> +      return false;
>> +
>>      HaveRewritablePHIs = true;
>>      ConstantExpr *OrigCE = dyn_cast<ConstantExpr>(OrigV);
>>      ConstantExpr *ThenCE = dyn_cast<ConstantExpr>(ThenV);
>> 
>> Modified: llvm/trunk/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/UnreachableEliminate.ll?rev=219462&r1=219461&r2=219462&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/SimplifyCFG/UnreachableEliminate.ll (original)
>> +++ llvm/trunk/test/Transforms/SimplifyCFG/UnreachableEliminate.ll Thu Oct  9 20:27:02 2014
>> @@ -71,3 +71,50 @@ U:
>>  T:
>>          ret i32 2
>>  }
>> +
>> +
>> +;; We can either convert the following control-flow to a select or remove the
>> +;; unreachable control flow because of the undef store of null. Make sure we do
>> +;; the latter.
>> +
>> +define void @test5(i1 %cond, i8* %ptr) {
>> +
>> +; CHECK-LABEL: test5
>> +; CHECK: entry:
>> +; CHECK-NOT: select
>> +; CHECK:  store i8 2, i8* %ptr
>> +; CHECK:  ret
>> +
>> +entry:
>> +  br i1 %cond, label %bb1, label %bb3
>> +
>> +bb3:
>> + br label %bb2
>> +
>> +bb1:
>> + br label %bb2
>> +
>> +bb2:
>> +  %ptr.2 = phi i8* [ %ptr, %bb3 ], [ null, %bb1 ]
>> +  store i8 2, i8* %ptr.2, align 8
>> +  ret void
>> +}
>> +
>> +; CHECK-LABEL: test6
>> +; CHECK: entry:
>> +; CHECK-NOT: select
>> +; CHECK:  store i8 2, i8* %ptr
>> +; CHECK:  ret
>> +
>> +define void @test6(i1 %cond, i8* %ptr) {
>> +entry:
>> +  br i1 %cond, label %bb1, label %bb2
>> +
>> +bb1:
>> +  br label %bb2
>> +
>> +bb2:
>> +  %ptr.2 = phi i8* [ %ptr, %entry ], [ null, %bb1 ]
>> +  store i8 2, i8* %ptr.2, align 8
>> +  ret void
>> +}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>