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

[Jingyue Wu]

Hi Duncan,

http://reviews.llvm.org/differential/diff/11958/ is the diff against the
latest version in D4659. The major change is that the new code uses
SimpilfyInstruction instead of foldPHINodeOrSelecInst.

I looked at why
>    %cond105.in.i.i = select i1 undef, float* null, float* undef
>    %cond105.i.i = load float* %cond105.in.i.i, align 8
is generated.

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".

Thanks,
Jingyue

On Sun Jul 27 2014 at 10:38:37 AM Duncan Sands <duncan.sands at gmail.com>
wrote:

> 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.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140728/96b29bea/attachment.html>