ChuanqiXu added a reviewer: dblaikie. ChuanqiXu added inline comments.
================ Comment at: clang/docs/CPlusPlus20Modules.rst:31 + +This document was intended to be a manual first and foremost, however, we consider it helpful to +introduce some language background here for readers who are not familiar with ---------------- dblaikie wrote: > The `was` here is intended. I want to say I **thought** to not add the all other information and just write it like a manual, which might look like: ``` -unwindlib=<value> Unwind library to use -U <macro> Undefine macro <macro> --verify-debug-info Verify the binary representation of debug output -verify-pch Load and verify that a pre-compiled header file is not stale --version Print version information -v Show commands to run and use verbose output -Wa,<arg> Pass the comma separated arguments in <arg> to the assembler -Wdeprecated Enable warnings for deprecated constructs and define __DEPRECATED -Wl,<arg> Pass the comma separated arguments in <arg> to the linker -working-directory <value> Resolve file paths relative to the specified directory -Wp,<arg> Pass the comma separated arguments in <arg> to the preprocessor -W<warning> Enable the specified warning -w Suppress all warnings -Xanalyzer <arg> Pass <arg> to the static analyzer -Xarch_device <arg> Pass <arg> to the CUDA/HIP device compilation -Xarch_host <arg> Pass <arg> to the CUDA/HIP host compilation -Xassembler <arg> Pass <arg> to the assembler -Xclang=<arg> Alias for -Xclang -Xclang <arg> Pass <arg> to clang -cc1 -Xcuda-fatbinary <arg> Pass <arg> to fatbinary invocation -Xcuda-ptxas <arg> Pass <arg> to the ptxas assembler -Xlinker <arg> Pass <arg> to the linker -Xoffload-linker<triple> <arg> Pass <arg> to the offload linkers or the ones idenfied by -<triple> -Xopenmp-target=<triple> <arg> Pass <arg> to the target offloading toolchain identified by <triple>. -Xopenmp-target <arg> Pass <arg> to the target offloading toolchain. -Xpreprocessor <arg> Pass <arg> to the preprocessor -x <language> Treat subsequent input files as having type <language> -z <arg> Pass -z <arg> to the linker ``` ================ Comment at: clang/docs/CPlusPlus20Modules.rst:70-76 +* Primary module interface unit. + +* Module implementation unit. + +* Module partition interface unit. + +* Module partition implementation unit. ---------------- ruoso wrote: > ChuanqiXu wrote: > > ruoso wrote: > > > The terminology here is a bit different than what we've been building the > > > consensus on. Please take a look at [[ > > > https://github.com/cplusplus/modules-ecosystem-tr/blob/master/sourcefiles.tex#L19 > > > | sourcefiles.tex ]] (or section `[source.types]` in the [[ > > > https://github.com/cplusplus/modules-ecosystem-tr/files/9237071/iso_cpp_modules_ecosystem_technical_report.pdf > > > | rendered version ]] > > > > > > > > If there is a consensus already, we should follow. > > > > (BTW, I thought we'll discuss module things in SG2 but it looks like we're > > discussing them in SG15... my bad) > Yes, please consider joining us for the sg15 meetings, we've been working > through quite a few things related to C++ modules. (Yeah, I've subscribed the sg15 mailing list.) I've followed the terminology in the link except `importable unit`. In that link, `importable unit` may contain header units. But in this documentation, we want to split modules and header units. So I take `importable module unit` here. I guess it might not be a big deal. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:225 + +It is possible to generate a module file for an importable module unit by specifying the ``--precompile`` option. + ---------------- ruoso wrote: > ChuanqiXu wrote: > > ruoso wrote: > > > Likewise, here the term "Built Module Interface file", with the acronym > > > "BMI" is what we're generally using when talking about the generated file. > > Yeah, this is what I was confused in the chat. In my mind, "BMI" describes > > a compatible interface format like ABI (like Itanium ABI). In another > > words, a BMI could be compiled by compiler which follows the BMI standard > > (like clang and gcc both accepts Itanium ABI). But currrently, a module > > file couldn't be compiled by clang in different versions. > > > > So from my point of view, the term `module file` is more appropriate than > > `BMI` now. > BMI is definitely not as compatible as the ABI. I think there's some > confusion here of what the term BMI is. In this case the `pcm` file is the > BMI. Oh, it confuses me. In my mind, BMI is corresponding to ABI (although the format of BMI is not standardized) and the `.pcm` file is corresponding to `.o` files naturally (to me). So generally, we don't say a `.o` file is the ABI. And similar, it is weird to me to say the `.pcm` files is the BMI. I've added a sentence in the terminology section of module file to explain the term 'BMI'. I tried to copy it from the module-ecosystem-technical-report but I failed to find one. So I just put the simple explanation. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:312-321 +Note that **currently** the compiler doesn't consider inconsistent macro definition a problem. For example: + +.. code-block:: console + + $ clang++ -std=c++20 M.cppm --precompile -o M.pcm + # Inconsistent optimization level. + $ clang++ -std=c++20 -O3 -DNDEBUG Use.cpp -fprebuilt-module-path=. ---------------- dblaikie wrote: > this sort of aside might be best left for a separate part of the document - > an FAQ/side-notes (a footnote, perhaps?), etc to keep the rest of the > document more focussed? I guess it is better to remain them here. Since the sentences tries to say the macro definitions are not language options, which is consistent to the above paragraphs to me. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:347 + $ clang++ -std=c++20 M.cppm --precompile -o M.pcm + $ rm -f M.cppm + $ clang++ -std=c++20 Use.cpp -fmodule-file=M.pcm ---------------- dblaikie wrote: > Could probably skip the `-f`? In my system, when I run `rm M.cppm`, it asks `rm: remove regular file ‘M.cppm’?` and I need to enter `y` then it would remove the file actually. So it looks better to add `-f` to me. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:395-396 + +Roughly, this theory is correct. But the problem is that it is too rough. Let's see what actually happens. +For example, the behavior also depends on the optimization level, as we will illustrate below. + ---------------- dblaikie wrote: > I'm not sure I'm able to follow the example and how it justifies the rough > theory as inadequate to explain the motivation for modules - could you > clarify more directly (in comments, and then we can discuss how to word it) > what the motivation for this section is/what you're trying to convey? Let me answer the motivation first. The motivation comes from my personal experience. I feel like when most people heard modules, they would ask "how much speedup could we get"? And there are some other questions like "why does modules speedup the compilation?". So I guess the readers of the document may have similar questions and I try to answer it here. The complexity theory is correct but it may be too abstract to our users. Since the complexity theory is about the scaling. But for certain users, the scales of their codes are temporarily fixed. So when they try to use modules but find the speedup doesn't meet their expectation in O2. They may feel frustrated. And it doesn't work if I say, "hey, you'll get much better speedup if the your codes get 10x longer." I guess they won't buy in. So what I try to do here is to manage the user's expectation to avoid any misunderstanding. Following off is about the explanation. For example, there are `1` module interface and `10` users. There is a function `F` in the module interface and the function is used by every users. And let's say we need a `T` time to compile the function `F` and each users without the function `F`. In O0, the function `F` will get compiled completely once and get involved in the Sema part 10 times. Due to the Sema part is relatively fast and let's say the Sema part would take `0.1T`. Given we compile them serially, we need `12T` to compile the project. But if we are with optimizations, each function `F` will get involved in optimizations and IPO in every users. And these optimizations are most time-consuming. Let's say these optimizations will consume `0.8T`. And the time required will be `19T`. It is easy to say the we need `20T` to compile the project if we're using headers. So we could find the speedup with optimization is much slower. BTW, if we write the required time with variables, it will be `nT + mT + T*m*additional_compilation_part`. The `additional_compilation_part ` here corresponds to the time percentage of `Sema` or `Optimizations`. And since `T` and `additional_compilation_part ` are both constant. So if we write them in `O()` form, it would be `O(n+m)`. So the theory is still correct. ================ Comment at: clang/docs/CPlusPlus20Modules.rst:610 + +Another difference with modules is that we can't compile the module file. +It makes sense due to the semantics of header units, which are just like headers. ---------------- dblaikie wrote: > Might need some more words here - I guess this means "there is no .o for a > .pcm from a header unit" basically? > I guess this means "there is no .o for a .pcm from a header unit" basically? Yes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131388/new/ https://reviews.llvm.org/D131388 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits