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

Sat Oct 8 19:08:16 PDT 2022

ChuanqiXu added a comment.

In D134267#3844605 <https://reviews.llvm.org/D134267#3844605>, @ChuanqiXu wrote:

> In D134267#3815453 <https://reviews.llvm.org/D134267#3815453>, @dblaikie wrote:
>
>>> This also tries to fix the problem I raised a year ago: https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144
>>
>> I think this thread is fairly different from what this patch is proposing.
>>
>> @rsmith for discussion of adding effectively implicit modules support for C++20 modules - this seems like a pretty significant design choice, but something we (you and I, SG15, etc) have discussed before in terms of "how do we support 'Hello, World.'" situations without requiring something that amounts to a build system even for the simplest C++20 modules programs. Maybe this is it? But it surprises me a bit, and I don't think the linked thread is sufficiently relevant to draw conclusions about this direction.
>
> I don't understand your point a lot. From my point, the intention of the thread (https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144) are asking why the command line interfaces of clang are clearly more complex than gcc. And @rsmith replies that:
>
>> Currently, Clang lacks two important features here:
>>
>> 1. Produce a .pcm file and a .o file from a single compilation action.
>
> in https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144/12. And I think this is what this patch tries to do: (generate .pcm file and .o file in one single compilation action).
>
> In fact, this doesn't change the behavior a lot. Previously, when we tried to compile a `*.cppm` directly to `*.o`, the compiler will generate `*.pcm` automatically too! Although the `*.pcm` file is only generated in `/tmp/` files by default. And what this patch did is only to change the default location for `*.pcm` files from `/tmp` to `module cache path`. And it wouldn't affect the original 2 phase compilation model.

In D134267#3845177 <https://reviews.llvm.org/D134267#3845177>, @dblaikie wrote:

> In D134267#3844605 <https://reviews.llvm.org/D134267#3844605>, @ChuanqiXu wrote:
>
>> In D134267#3815453 <https://reviews.llvm.org/D134267#3815453>, @dblaikie wrote:
>>
>>>> This also tries to fix the problem I raised a year ago: https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144
>>>
>>> I think this thread is fairly different from what this patch is proposing.
>>>
>>> @rsmith for discussion of adding effectively implicit modules support for C++20 modules - this seems like a pretty significant design choice, but something we (you and I, SG15, etc) have discussed before in terms of "how do we support 'Hello, World.'" situations without requiring something that amounts to a build system even for the simplest C++20 modules programs. Maybe this is it? But it surprises me a bit, and I don't think the linked thread is sufficiently relevant to draw conclusions about this direction.
>>
>> I don't understand your point a lot. From my point, the intention of the thread (https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144) are asking why the command line interfaces of clang are clearly more complex than gcc. And @rsmith replies that:
>>
>>> Currently, Clang lacks two important features here:
>>>
>>> 1. Produce a .pcm file and a .o file from a single compilation action.
>>
>> in https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/59144/12. And I think this is what this patch tries to do: (generate .pcm file and .o file in one single compilation action).
>>
>> In fact, this doesn't change the behavior a lot. Previously, when we tried to compile a `*.cppm` directly to `*.o`, the compiler will generate `*.pcm` automatically too! Although the `*.pcm` file is only generated in `/tmp/` files by default. And what this patch did is only to change the default location for `*.pcm` files from `/tmp` to `module cache path`. And it wouldn't affect the original 2 phase compilation model.
>>
>> BTW, this can simplify the use of modules significantly, see https://reviews.llvm.org/D135507#change-gpsoCkM1g61J for example.
>
> The use of a module cache path I think is a pretty major difference between what was discussed on the discourse thread and what's being proposed here - a module "cache" is what starts to touch near Clang's old implicit modules that has real problems in terms of parallelism, etc. (granted what you're proposing here doesn't go all the way towards that - it doesn't have implicit actions to generate the pcm, at least)
>
> Is this what GCC Supports? I'd expect something more like Split DWARF, where the .pcm (or .dwo file) is placed next to the .o file - not in some separate directory (what sort of naming scheme would be used to ensure that paths in that directory are unique?).

Oh, I see. So you're not against the idea of producing a .pcm file and .o file in a single compilation. So we don't have any fundamental conflicts.

>> Is this what GCC Supports?

It looks similar to me. GCC will generate and search the bmi (they called it .cmi files) in the `{current_dir}/gcm.cache` directories automatically. But I don't find the option to specify the cache directory in the gcc manual. So I guess the path is fixed in gcc. The difference in this patch is that we allows the user to specify the position of the cache dir. Then I was wondering - if I want a path to store modules cache and there is an existing option `-fmodules-cache-path`, can I reuse it? - then here is the patch.

> a module "cache" is what starts to touch near Clang's old implicit modules that has real problems in terms of parallelism

Could you elaborate more on this? Do you mean it is bad that the C++20 Modules share the cache path with clang modules? Or is it bad about the idea of cache path?

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

https://reviews.llvm.org/D134267