Hi Duncan, <br><br><a href="http://reviews.llvm.org/differential/diff/11958/" target="_blank">http://reviews.llvm.org/differential/diff/11958/</a> is the diff against the latest version in D4659. The major change is that the new code uses SimpilfyInstruction instead of foldPHINodeOrSelecInst. <div>
<br></div><div>I looked at why</div><div>>    %cond105.in.i.i = select i1 undef, float* null, float* undef<br>>    %cond105.i.i = load float* %cond105.in.i.i, align 8</div><div>is generated. </div><div><br></div><div>
SimplifyInstruction did get kicked in, but the result of SimplifyInstruction  does not equal *U, the pointer use being tracked (namely %1=gep... in my example). Therefore, SROA pushes the select to S.DeadOperands, and later replaces "select undef, null, %1" with "select undef, null, undef". </div>
<div><br></div><div>Thanks,</div><div>Jingyue<br><br><div class="gmail_quote">On Sun Jul 27 2014 at 10:38:37 AM Duncan Sands <<a href="mailto:duncan.sands@gmail.com" target="_blank">duncan.sands@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Jingyue,<br>
<br>
On 27/07/14 18:21, Jingyue Wu wrote:<br>
> Hi Duncan,<br>
><br>
> I tried SimplifyPHINode and it worked pretty well. Thanks!<br>
><br>
> That makes me consider using SimplifySelectInst on select instructions too. However, I found one regression test PR16651.2 would fail after this potential modification. We would transform<br>
><br>
> ```<br>
> define void @PR16651.2() {<br>
> ; This test case caused a crash due to failing to promote given a select that<br>
> ; can't be speculated. It shouldn't be promoted, but we missed that fact when<br>
> ; analyzing whether we could form a vector promotion because that code didn't<br>
> ; bail on select instructions.<br>
> ;<br>
> ; CHECK-LABEL: @PR16651.2(<br>
> ; CHECK: alloca <2 x float><br>
> ; CHECK: ret void<br>
><br>
> entry:<br>
>    %tv1 = alloca { <2 x float>, <2 x float> }, align 8<br>
>    %0 = getelementptr { <2 x float>, <2 x float> }* %tv1, i64 0, i32 1<br>
>    store <2 x float> undef, <2 x float>* %0, align 8<br>
>    %1 = getelementptr inbounds { <2 x float>, <2 x float> }* %tv1, i64 0, i32 1, i64 0<br>
>    %cond105.in.i.i = select i1 undef, float* null, float* %1<br>
>    %cond105.i.i = load float* %cond105.in.i.i, align 8<br>
>    ret void<br>
> }<br>
> ```<br>
> to<br>
> ```<br>
> define void @PR16651.2() {<br>
> entry:<br>
>    %cond105.in.i.i = select i1 undef, float* null, float* undef<br>
>    %cond105.i.i = load float* %cond105.in.i.i, align 8<br>
>    ret void<br>
> }<br>
> ```<br>
><br>
> Is this transformation on PR16651.2 valid? If no, can somebody help me understand why it isn't?<br>
<br>
I don't understand what happened here.  If SimplifySelectInst simplifies a<br>
select then it replaces it with one of the values it can take (none of the<br>
simplifications in InstructionSimplify ever creates new instructions, they only<br>
return things that were already there; well, they may create new *constants* but<br>
not new instructions), here the select should have disappeared and been replaced<br>
by either float* null or float* %1.  Given that SimplifySelectInst prefers<br>
constants, it would have been float* null.  Yet the select is still there and<br>
being used, and all the other transformations are very<br>
un-InstructionSimplify-like, which says to me that SimplifySelectInst didn't<br>
kick in at all.  Maybe you should share your patch for making use of<br>
SimplifySelectInst?<br>
<br>
Ciao, Duncan.<br>
______________________________<u></u><u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailm<u></u>an/listinfo/llvm-commits</a><br>
</blockquote></div></div>