[PATCH] D12066: Introduce the ScopExpander as a SCEVExpander replacement

[hfinkel@anl.gov via llvm-commits]

hfinkel added a comment.

In http://reviews.llvm.org/D12066#225379, @grosser wrote:

> > > The SCEVExpander cannot deal with all SCEVs Polly allows in all kinds
>
> > 
>
> > 
>
> > 
>
> > 
>
> > >   of expressions. To this end we introduce a ScopExpander that handles
>
> > 
>
> > 
>
> > 
>
> > 
>
> > >   the additional expressions separatly and falls back to the
>
> > 
>
> > 
>
> > 
>
> > 
>
> > >   SCEVExpander for everything else.
>
> > 
>
> > 
>
> > Shouldn't you just enhance SCEVExpander?
>
>
> Hi Hal,
>
> thanks for jumping in.


No problem ;)

> The point is that Polly starts to support thinks that go beyond what we

>  expression in SCEV generally. The current expressions we are facing, sDiv and sRem, could probably be added

>  to SCEV and consequently the SCEVExpander as well, but the next step will be piecewise expressions

>  such as a < b ? c : d. I am less convinced this really needs to be added to SCEV. Or at least,

>  I feel that we better keep this in Polly for a while, gain some experience, and move it if the

>  added value of this becomes really visible. In general I am trying to push as much of the stuff we

>  are doing directly into LLVM, but at the same time I feel we need to make sure we don't create too

>  much code that is only useful for Polly ATM.

> 

> I would be interested if you agree/disagree with this reasoning.


How are you representing these new SCEV-like nodes? This code seems to look though SCEV unknowns that happen to be sdiv/srem instructions and re-expand the operands. I don't understand why the operands would not otherwise be available (unless these are uninserted instructions -- in which case, I'd think that adding more SCEV subclasses would be a better design -- but maybe I'm overlooking some downside to that).

> Best,

> Tobias





http://reviews.llvm.org/D12066