[PATCH] D34444: Teach codegen to work in incremental processing mode.

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 21 21:23:22 PDT 2017


rjmccall added a comment.

In https://reviews.llvm.org/D34444#812836, @v.g.vassilev wrote:

> In https://reviews.llvm.org/D34444#812418, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D34444#795175, @v.g.vassilev wrote:
> >
> > > @rjmccall, thanks for the prompt and thorough reply.
> > >
> > > In https://reviews.llvm.org/D34444#793311, @rjmccall wrote:
> > >
> > > > Okay.  In that case, I see two problems, one major and one potentially major.
> > >
> > >
> > >
> > >
> > >   This is a very accurate diagnosis which took us 5 years to discover on an empirical basis ;)
> >
> >
> > You could've asked at any time. :)
>
>
> True. I am not really sure I knew what to ask, though ;)


We're open to general "I'm trying to do this and having problems" questions on the mailing lists.  You probably would've needed to know to CC me specifically, though; sadly, I can't keep up with all the lists I need to.

>> That's quite brittle, because that code is only executed in a code path that only you are using, and you're not adding any tests.  I would greatly prefer a change to IRGen's core assumptions, as suggested.
> 
> I am open to changing this code as well. That should probably be another review.

I agree.  Are you comfortable with blocking this review until that lands?  It seems like it would significantly change this.

>> I feel it is important that there be a way to inform an ASTConsumer that no further requests will be made of it, something other than calling its destructor.  I would like you to make sure that the ASTConsumer interface supports that and that that call is not made too soon in your alternate processing mode.
> 
> Do you have a preference of a name of this new interface?

Maybe just "finish"?

John.


Repository:
  rL LLVM

https://reviews.llvm.org/D34444





More information about the cfe-commits mailing list