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

[Arnold Schwaighofer]

> On Oct 10, 2014, at 12:14 PM, Philip Reames <listmail at philipreames.com> wrote:
> 
> 
> On 10/10/2014 12:05 PM, Arnold Schwaighofer wrote:
>> 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 ...
> This would be my preference.  I dislike the coupling between optimizations created by your patch.  On first look, it seems like your patch would be unnecessary if we handled the select case.  Is this correct, or are their other cases you're aware of?

I think, the more general thing is to remove the control flow (not to create the select that we later remove - leaving instructions that we would have otherwise removed). This handles both:

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

bb1:
  br label %bb2

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

and this

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

bb1:
  call @foobar()
  br label %bb2

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

Also, we would have to teach whatever optimization to fix up all select users of a undef select condition decision (not hard but still redoing work we are already doing in simplifycfg), instcombine is typically not doing this sort of optimization.


define void @testcase(i1 %cond, i8* %ptr, i32 * %ptrv, i32 %v) {
entry:
  br i1 %cond, label %bb1, label %bb2

bb1:
 %v2 = add i32 %v, 10
  br label %bb2


bb2:
  %ptr.2 = phi i8* [ %ptr, %entry], [ null, %bb1 ]
  %v.3 = phi i32 [ %v, %entry ],   [ %v2, %bb1 ]
  store i8 2, i8* %ptr.2
  store i32 %v.3 , i32* %ptrv
  ret void

}

opt -simplifycfg (before my patch):

define void @testcase(i1 %cond, i8* %ptr, i32* %ptrv, i32 %v) {
entry:
  %v2 = add i32 %v, 10
  %.ptr = select i1 %cond, i8* null, i8* %ptr
  %v2.v = select i1 %cond, i32 %v2, i32 %v
  store i8 2, i8* %.ptr
  store i32 %v2.v, i32* %ptrv
  ret void
}

SimplifyCFG should not be creating the select to begin with in my opinion. I don’t see a coupling here. I see it the other way round if you generated the select in simplifycfg, someone (instcombine) has to clean it up. 

To me, the argument for also handling selects is that some else might generate a select %cond, null ...




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141010/bd3b70c1/attachment.html>