[PATCH] D16403: Add scope information to CFG

Maxim Ostapenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 20 09:39:07 PST 2018


m.ostapenko added a comment.

In https://reviews.llvm.org/D16403#1011218, @NoQ wrote:

> In https://reviews.llvm.org/D16403#1010096, @szepet wrote:
>
> > In https://reviews.llvm.org/D16403#992454, @NoQ wrote:
> >
> > > @szepet: so i see that `LoopExit` goes in the beginning of the cleanup block after the loop, while various `ScopeEnd`s go after the `LoopExit`. Would loop unrolling be significantly broken if you simply subscribe to `ScopeEnd` instead of `LoopExit` and avoid cleaning up the loop state until destructors are processed? I might not be remembering correctly - is `LoopExit` only used for cleanup, or do we have more work to be done here?
> >
> >
> > I guess your following comment just answers this:
> >
> > In https://reviews.llvm.org/D16403#1001466, @NoQ wrote:
> >
> > > We still don't have scopes for segments of code that don't have any variables in them, so i guess it's not yet in the shape where it is super useful for loops, but it's already useful for finding use of stale stack variables, which was the whole point originally, so i think this should definitely land soon.
> >
> >
> > It could be, however, we would lose cases like:
> >
> >   int i = 0;
> >   int a[32];
> >   for(i = 0;i<32;++i) {a[i] = i;}
> >
> >
> > Since there is no variable which has the scope of the loop, ScopeEnd would be not enough. Sure, we could remove this case, however, the aim is to extend the loop-patterns for completely unrolling. Another thing that there are the patches which would enhance the covered cases by LoopExit (https://reviews.llvm.org/D39398) and add the LoopEntrance to the CFG(https://reviews.llvm.org/D41150) as well. So, at this point, I feel like it would be a huge step back not to use these elements. (Sorry Maxim that we are discussing this here^^)
>
>
> Yeah, i mean, like, if we change the scope markers to also appear even when no variables are present in the scope, then it would be possible to replace loop markers with some of the scope markers, right?


Hm, so does this mean that I need to cover the case when no variables are present in loop scope here in this patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403





More information about the llvm-commits mailing list