[clang] Llvm modules on demand bmi (PR #71773)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

> > There are 2 things in the patch. One is to generate the BMI and the object file in one phase (phase here means preprocess, precompile, compile, ...).
> 
> This is the main point of the patch - to do this efficiently.

Got it. The we can be more focused.

> 
> > But after we introduced thin BMI, it looks inefficient to write the AST twice. So it is on my TODO list after we land the thin BMI patch. BTW, I think we should do thin in CodeGen action instead of hacking on WrappedASTConsumer.
> 
> I am curious as to why you think that the multiplex AST consumer is a hack - it seems to be designed exactly for this purpose and existed already (i.e. not part of this patch).

It is not about multiplex AST consumer. It is about WrappedASTConsumer. It is designed for plugins. Also it is a private member function of FrontendAction, the base of frontend actions. I think we should perform new behaviors in sub-actions. It looks not good to perform semantical analysis in FrontendAction...

Concretely, I think we need to do this in CodeGenAction.

> 
> > And if we introduce the mechanism to produce BMI for `.cpp`, it implies that we need to maintain both paths. It is super embracing to me.
> 
> We do not need two mechanisms, .cppm can take the same path as any other suffix.

Then it implies that we need to discard a bunch of existing codes handling `.cppm`. Otherwise we'll have two mechanisms.

> 
> > > in the AST consumer on the BMI side doing suitable filtering to eliminate the content that is not part of the interface, that is either not needed (or in some cases positively unhelpful to consumers).
> 
> > I believe we should do this in ASTWriters.
> 
> I am strongly against doing more semantic work in the AST reader/writer; that is just compounding existing layering violations that are already hurting us.

Agreed in the higher level. But that requires us to implement at least new AST writers.

> 
> > Also this should be part of thin BMI.
> 
> I am not sure what you mean here - the full AST is required for code-gen - we can only thin AST either on a separate path (as in this patch) or as a separate step.

I mean it should be successors of https://github.com/llvm/llvm-project/pull/71622. Concretely, now we reduce the function definition in https://github.com/llvm/llvm-project/pull/71622/files#diff-125f472e690aa3d973bc42aa3c5d580226c5c47661551aca2889f960681aa64dR321.



https://github.com/llvm/llvm-project/pull/71773