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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In https://reviews.llvm.org/D34444#818030, @rjmccall wrote:

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


Good to know. Thanks! I will come back to you once I get rid of our O(100) clang patches to discuss what would be the best way of supporting incremental compilation.

>>> 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 am afraid that will slow down (if not suspend) our efforts to upstream our local patches. This patch is pretty fundamental for cling and if we change it now, I will have to go back and rework our implementation. I'd be much more comfortable in reworking it once we run on vanilla clang (then efforts seems easier to justify on our end).

It seems to me that despite being suboptimal, it is not very intrusive and it would effect only on our use case. I can keep track of such patches and come back to you for advice how to do them best. Would that make sense?

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