[PATCH] [SROA] Fold a PHI node if all its incoming values are the same

[Duncan Sands]

Hi Jingyue,

On 27/07/14 18:21, Jingyue Wu wrote:
> Hi Duncan,
>
> I tried SimplifyPHINode and it worked pretty well. Thanks!
>
> 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
>
> ```
> define void @PR16651.2() {
> ; This test case caused a crash due to failing to promote given a select that
> ; can't be speculated. It shouldn't be promoted, but we missed that fact when
> ; analyzing whether we could form a vector promotion because that code didn't
> ; bail on select instructions.
> ;
> ; CHECK-LABEL: @PR16651.2(
> ; CHECK: alloca <2 x float>
> ; CHECK: ret void
>
> entry:
>    %tv1 = alloca { <2 x float>, <2 x float> }, align 8
>    %0 = getelementptr { <2 x float>, <2 x float> }* %tv1, i64 0, i32 1
>    store <2 x float> undef, <2 x float>* %0, align 8
>    %1 = getelementptr inbounds { <2 x float>, <2 x float> }* %tv1, i64 0, i32 1, i64 0
>    %cond105.in.i.i = select i1 undef, float* null, float* %1
>    %cond105.i.i = load float* %cond105.in.i.i, align 8
>    ret void
> }
> ```
> to
> ```
> define void @PR16651.2() {
> entry:
>    %cond105.in.i.i = select i1 undef, float* null, float* undef
>    %cond105.i.i = load float* %cond105.in.i.i, align 8
>    ret void
> }
> ```
>
> Is this transformation on PR16651.2 valid? If no, can somebody help me understand why it isn't?

I don't understand what happened here.  If SimplifySelectInst simplifies a 
select then it replaces it with one of the values it can take (none of the 
simplifications in InstructionSimplify ever creates new instructions, they only 
return things that were already there; well, they may create new *constants* but 
not new instructions), here the select should have disappeared and been replaced 
by either float* null or float* %1.  Given that SimplifySelectInst prefers 
constants, it would have been float* null.  Yet the select is still there and 
being used, and all the other transformations are very 
un-InstructionSimplify-like, which says to me that SimplifySelectInst didn't 
kick in at all.  Maybe you should share your patch for making use of 
SimplifySelectInst?

Ciao, Duncan.