<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><blockquote type="cite" class="">On Oct 10, 2014, at 12:14 PM, Philip Reames <<a href="mailto:listmail@philipreames.com" class="">listmail@philipreames.com</a>> wrote:<br class=""><br class=""><br class="">On 10/10/2014 12:05 PM, Arnold Schwaighofer wrote:<br class=""><blockquote type="cite" style="font-family: Menlo-Regular; font-size: 11px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">Right, however the code is already there and also handles the more general:<br class=""><br class=""> define void @test6(i1 %cond, i8* %ptr) {<br class=""> entry:<br class=""> br i1 %cond, label %bb1, label %bb2<br class=""><br class=""> bb1:<br class=""> call void foobar()<br class=""> br label %bb2<br class=""><br class=""> bb2:<br class=""> %ptr.2 = phi i8* [ %ptr, %entry ], [ null, %bb1 ]<br class=""> store i8 2, i8* %ptr.2, align 8<br class=""> ret void<br class=""> }<br class=""><br class="">=><br class=""><br class=""> define void @test6(i1 %cond, i8* %ptr) {<br class=""> entry:<br class=""> br i1 %cond, label %bb1, label %bb2<br class=""><br class=""> bb1:<br class=""> call void foobar()<br class=""> unreachable<br class=""><br class=""> bb2:<br class=""> store i8 2, i8* %ptr, align 8<br class=""> ret void<br class=""> }<br class=""><br class="">There is an argument to be made to do both I guess ...<br class=""></blockquote>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?<br class=""></blockquote><br class="">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:<br class=""><br class="">define void @test6(i1 %cond, i8* %ptr) {<br class="">entry:<br class=""> br i1 %cond, label %bb2, label %bb1<br class=""><br class="">bb1:<br class=""> br label %bb2<br class=""><br class="">bb2:<br class=""> %ptr.2 = phi i8* [ %ptr, %entry ], [ null, %bb1 ]<br class=""> store i8 2, i8* %ptr.2, align 8<br class=""> ret void<br class="">}<br class=""><br class="">and this<br class=""><br class="">define void @test6(i1 %cond, i8* %ptr) {<br class="">entry:<br class=""> br i1 %cond, label %bb2, label %bb1<br class=""><br class="">bb1:<br class=""> call @foobar()<br class=""> br label %bb2<br class=""><br class="">bb2:<br class=""> %ptr.2 = phi i8* [ %ptr, %entry ], [ null, %bb1 ]<br class=""> store i8 2, i8* %ptr.2, align 8<br class=""> ret void<br class="">}<br class=""><br class="">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.<br class=""><br class=""><br class=""><div class="">define void @testcase(i1 %cond, i8* %ptr, i32 * %ptrv, i32 %v) {<br class="">entry:<br class=""> br i1 %cond, label %bb1, label %bb2<br class=""><br class="">bb1:<br class=""> %v2 = add i32 %v, 10<br class=""> br label %bb2<br class=""><br class=""><br class="">bb2:<br class=""> %ptr.2 = phi i8* [ %ptr, %entry], [ null, %bb1 ]<br class=""> %v.3 = phi i32 [ %v, %entry ], [ %v2, %bb1 ]<br class=""> store i8 2, i8* %ptr.2<br class=""> store i32 %v.3 , i32* %ptrv<br class=""> ret void<br class=""><br class="">}<br class=""><br class="">opt -simplifycfg (before my patch):</div><div class=""><br class="">define void @testcase(i1 %cond, i8* %ptr, i32* %ptrv, i32 %v) {<br class="">entry:<br class=""> %v2 = add i32 %v, 10<br class=""> %.ptr = select i1 %cond, i8* null, i8* %ptr<br class=""> %v2.v = select i1 %cond, i32 %v2, i32 %v<br class=""> store i8 2, i8* %.ptr<br class=""> store i32 %v2.v, i32* %ptrv<br class=""> ret void<br class="">}<br class=""><br class="">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. <div class=""><br class=""></div><div class="">To me, the argument for also handling selects is that some else might generate a select %cond, null ...<br class=""><br class=""><br class=""><br class=""><br class=""></div></div></body></html>