[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D134267#3847399 <https://reviews.llvm.org/D134267#3847399>, @iains wrote:

> FAOD: I think this should not be about "who's patches" - but about the compilation model and command lines that we want to end up with.

Yeah, of course.

> I am concerned that with the current scheme (with or without the change in this review) we could end up with three process launches per modular file:
>
> - one to determine the deps
> - one to produce the BMI
> - one to produce the object
>
> At least with GCC (and a patch series like the one I drafted) we can reduce that to 2
>
> With the interfaces mentioned in P1184 <https://reviews.llvm.org/P1184>, it is plausible to make that 1 (at the expense of potentially many stalled builds - but stalled early in compilation, so hopefully with the smallest memory footprint we could achieve).

I pretty believe the current scheme is better for the following reasons:
(1) The current scheme has higher paralellism potentially. See the `Why two-phase compilation?` section in https://reviews.llvm.org/D134269 for examples. I believe this should be the future direction to speedup the compilation.
(2) Your scheme saves the time for deserialization. However, clang implement many ODR checks in the deserialization part. Skipping the deserialization will omit these ODR checks, which will break the semantics. And this is the reason why that scheme may not be good.
(3) It makes implementation more complex.

For the first 2 reasons, I believe clang is in the right directions. Also for the filename suffixes, it is helpful for static analyzing tools to know whether or not if a file is module unit. So the static analyzing tools can scan every `.cppm` file in the working directory automatically in the preparation time. For example, if the build system are ready enough, we can omit the `module files` in the build script like: https://github.com/ChuanqiXu9/llvm-project/blob/94a71a7046a8672a7e06311b0c05c0a8c9ae4619/P1689Examples/HelloWorld/CMakeLists.txt#L18-L19.
I believe this is the reason why MSVC made a similar choice. (clang is easy to support MSVC's 'ixx' extension).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134267/new/

https://reviews.llvm.org/D134267