[PATCH] D38494: [SCEV] Handling for ICmp occuring in the evolution chain.

[Jun Ryung Ju via Phabricator via llvm-commits]

junryoungju added a comment.

In https://reviews.llvm.org/D38494#911512, @jbhateja wrote:

> In https://reviews.llvm.org/D38494#911501, @junryoungju wrote:
>
> > The problem is, if under ICmp's operand node has a ICmp can evolutable? (or any other conditional expressions).
> >  This only can be solved by registering this chain into getSCEV.
> >
> >   ; RUN: opt -analyze -scalar-evolution < %s | FileCheck %s --check-prefix=CHECK-ANALYSIS
> >  
> >   define i32 @foo() {
> >     br label %do_start
> >  
> >   do_start:
> >     %inc = phi i32 [ 0, %0 ], [ %add, %do_start ]
> >     %cmp = icmp slt i32 %inc, 10000
> >     %add_cond = add nsw i32 %inc, 2
> >     %sel = select i1 %cmp, i32 %add_cond, i32 %inc
> >     %add = add nsw i32 %sel, 1
> >     br i1 %cmp, label %do_start, label %do_end
> >  
> >   ; CHECK-ANALYSIS: Loop %do_start: backedge-taken count is 3334
> >   ; CHECK-ANALYSIS: Loop %do_start: max backedge-taken count is 3334
> >   ; CHECK-ANALYSIS: Loop %do_start: Predicated backedge-taken count is 3334
> >  
> >   do_end:
> >     ret i32 0
> >   }
> >
> >
> > This case is "actaully" generated from Clang executable using `-O3 -S -emit-llvm` option.
> >  that mean this case can be "always" appear.
> >
> > To fix this case, we need to create SCEVConditional. that I commented here.
> >
> > In https://reviews.llvm.org/D38494#908808, @junryoungju wrote:
> >
> > > Okay, just we need to make sure we are evoluting conditional instructions or just checking that instructions are true or false for now.
> > >
> > > first one.
> > >  If we are "actaully" evoluting a conditional instructions. better creating SCEVConditonal, checking that is true or false when is computing backedge taken count on each AddRec step.
> > >  I think this is a correct way for handling conditional instructions.
> > >  but, this will cause SCEV jobs MUUUUCHHH SLOWER.
> > >
> > > second one.
> > >  If we are just checking that instructions are true or false. we do not need to rewrite it, rewriting value means that we handle that value as that "actual" SCEV. (register SCEV on SCEVCallbackVH)
> > >  this mean, this is same to write this chain on to createSCEV that we did at first.
> > >  so we have to handle it as a "except" of SCEV chain. better inlining code into createAddRecFromPHI.
> >
> >
> > and just note this issue CANNOT be solved just by checking ICmp has conditional instruction.
>
>
> Just try pulling out earlier diff : https://reviews.llvm.org/D38494?id=120193 , this was fixing your new case.
>  You shuld attempt an incremental fix once this patch goes through.


That isn't a POINT, Please read the point that I commented.
Only that handling each test case dosn't means fixing a "point" of problem.

The point is are we handling the problem just by CHECKING that ICmp is true, or false.
OR are we REALLY wants to handle condtional instructions.
The point of problem is, current SCEV chain cannot handle conditonal expressions.
NOT only a clang, clang is emiiting by PHI node, LLVM is a toolchain that we can create a compiler.

so I wanted to listen the opinion that "how about handling condtional variables by just creating SCEVCondtional, and comparing at computing backedge-count by visiting each step."


https://reviews.llvm.org/D38494