<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 10/10/2014 01:35 PM, Arnold
Schwaighofer wrote:<br>
</div>
<blockquote
cite="mid:FB26D666-1856-4664-B8D9-02A9B079CB2D@apple.com"
type="cite"><br class="">
<blockquote type="cite" class="">On Oct 10, 2014, at 12:14 PM,
Philip Reames <<a moz-do-not-send="true"
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="">
</blockquote>
I see your point here, but we should be able to trigger the select
optimization without loosing information. If we do, that's
concerning. <br>
<br>
p.s. I want to be clear here. I'm talking about ideals, not
practicality. Your change is fine as is, I'm just debating what the
'best' approach would be. <br>
<blockquote
cite="mid:FB26D666-1856-4664-B8D9-02A9B079CB2D@apple.com"
type="cite"><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="">
</blockquote>
I was viewing this a bit differently. I was picturing a pattern
which started from the store, and looked to see if it's pointer
input was a select with a null input. If so, it would replace the
select with the other input. Potentially, it could also exploit the
implied condition (in a path sensitive manner)<br>
<blockquote
cite="mid:FB26D666-1856-4664-B8D9-02A9B079CB2D@apple.com"
type="cite"><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. <br>
</div>
</blockquote>
What if a frontend creates the select? We should handle that case
regardless of what the optimizer creates. Given we have to handle
it, I don't see the value in avoiding creating it elsewhere in the
optimizer. <br>
<blockquote
cite="mid:FB26D666-1856-4664-B8D9-02A9B079CB2D@apple.com"
type="cite">
<div class="">
<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="">
</div>
</div>
</blockquote>
I think we're in agreement here. <br>
<blockquote
cite="mid:FB26D666-1856-4664-B8D9-02A9B079CB2D@apple.com"
type="cite">
<div class="">
<div class=""><br class="">
<br class="">
<br class="">
<br class="">
</div>
</div>
</blockquote>
<br>
</body>
</html>