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

Mon Oct 10 02:42:56 PDT 2022

iains added a comment.

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

> In D134267#3846218 <https://reviews.llvm.org/D134267#3846218>, @ChuanqiXu wrote:
>
>> In D134267#3846153 <https://reviews.llvm.org/D134267#3846153>, @iains wrote:
>>
>>> @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in progress here;
>>>
>>> I have implemented a similar scheme to GCC's where C/BMIs can be produced "on demand" when the FE encounters the module keywords.  The idea was to make it possible to have the same command lines on both compilers (basically to remove the need for the user to identify that a c++ source is is modular - with the sole exception of header units which the compiler cannot figure out on its own).
>>>
>>> This scheme interfaces with the module-mapper approach (but how the mapping is determined could just as easily be from command line flags, module map files etc. etc. that is the beauty of that approach - the compiler has no need to know about how the mapping is made - it can be a fixed in-process scheme (like using the module cache path + some well-known mapping of names) or a full-blown build system - e.g. attaching to build2 (I think Boris did try out an earlier version of my implementation.)
>>>
>>> draft patches (need rebasing) here: D118896 <https://reviews.llvm.org/D118896>, D118898 <https://reviews.llvm.org/D118898>,D118895 <https://reviews.llvm.org/D118895>, D118899 <https://reviews.llvm.org/D118899> (these do not include the libCody stuff, which would support the module mapper, but concentrate on the compiler side).
>>> edit: D118895 <https://reviews.llvm.org/D118895> is probably no longer relevant.

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

> In D134267#3846264 <https://reviews.llvm.org/D134267#3846264>, @iains wrote:
>
>> In D134267#3846218 <https://reviews.llvm.org/D134267#3846218>, @ChuanqiXu wrote:
>>
>>> In D134267#3846153 <https://reviews.llvm.org/D134267#3846153>, @iains wrote:
>>>
>>>> @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in progress here;
>>>>
>>>> I have implemented a similar scheme to GCC's where C/BMIs can be produced "on demand" when the FE encounters the module keywords.  The idea was to make it possible to have the same command lines on both compilers (basically to remove the need for the user to identify that a c++ source is is modular - with the sole exception of header units which the compiler cannot figure out on its own).
>>>>
>>>> This scheme interfaces with the module-mapper approach (but how the mapping is determined could just as easily be from command line flags, module map files etc. etc. that is the beauty of that approach - the compiler has no need to know about how the mapping is made - it can be a fixed in-process scheme (like using the module cache path + some well-known mapping of names) or a full-blown build system - e.g. attaching to build2 (I think Boris did try out an earlier version of my implementation.)
>>>>
>>>> draft patches (need rebasing) here: D118896 <https://reviews.llvm.org/D118896>, D118898 <https://reviews.llvm.org/D118898>,D118895 <https://reviews.llvm.org/D118895>, D118899 <https://reviews.llvm.org/D118899> (these do not include the libCody stuff, which would support the module mapper, but concentrate on the compiler side).
>>>> edit: D118895 <https://reviews.llvm.org/D118895> is probably no longer relevant.
>>>>
>>>> Reworking/rebasing this scheme and posting the patches is next on my TODO after (current work on) P1815 <https://reviews.llvm.org/P1815> part 2.
>>>>
>>>> Hopefully, there is some way to combine these pieces of work to give the user greatest flexibility.
>>>
>>> @iains oh, I remembered just now that you said you'd like to take this in one of GitHub issues long ago. My bad. I forgot it when I started this. I think you can add me as subscriber such drafts next time to avoid similar things happen again.
>>> In fact, I had some drafts about modules and they conflicted with yours previously. So I'm sure I understand this : (
>>>
>>> And for this concrete job (generate pcm and object file in one compilation job), I think my implementation looks better. It is shorter and reuses the current behavior (generate `.pcm` files in `/tmp` directories and we just redirect it). So I think it may be better to pursue this patch for this task. (The option design could be changed of course).
>>
>> I need to review this patch some more - but at first looking it seems to be driver-only - so I am not sure how you are arranging for the PCM  **and** the object to  be created from a single invocation of the FE.  The idea of my patch series is to match the behavior o GCC - where the FE can figure out (independently of any file suffix) that it needs to emit an BMI.
>
> Let me add some more words to ease the understand. The secret here is that clang knows it is compiling a module unit before parsing - by the suffix `.cppm` or specified `-xc++-module` explicitly. And the clang would always generate the pcm file and the object file for the module unit in the past time too. The original behavior to compile a `*.cppm` to `*.o` would be:
>
> 1. Compile `*.cppm` to `/tmp/random-name.pcm`
> 2. Compile `/tmp/random-name.pcm` to `*.o`
>
> And the only change this patch made is to change the destination of `/tmp/random-name.pcm`.
>
> BTW, the meaning of `--precompile` (or `-emit-module-interface` in Xclang) is to stop at the precompilation step instead of doing precompilation. Since precompilation would always happen for module unit.

OK, that is clear then,  here is the main difference in approach (and that reflects the difference between the GCC scheme and the current clang one).

for my patch set:

1. the requirement for special names (.cppm ) or special tags -xc++-module for the driver is removed, since the FE will be able to determine that a file is a modular one and produce the BMI as needed.
2. The BMI is not produced and moved, it is produced //only// when required and in place.
3. there is only **//one//** invocation of the FE, which produces both artefacts.

Now, of course, both approaches are reasonable - but for my 0.02GBP I agree with several folks who would say that the user should not have to use special filename extensions for  "standard C++".

<snip> second part for now we can come back to that as needed.

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

https://reviews.llvm.org/D134267