Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ C++20 Feature Support
will now work.
(#GH62925).

- Clang refactored the BMI format to make it possible to support no transitive changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explicitly mention the user-facing effect of what "no transitive change" means and mention that we want feedback on this?

Something like:

  • Clang implemented improvements to BMI of C++20 Modules that should reduce the number of rebuilds during incremental recompilation. We are seeking feedback from Build System authors and other interested users, especially when you feel Clang changes the BMI and missses an opportunity to avoid recompilations or causes correctness issues. See StandardCPlusPlusModules <StandardCPlusPlusModules.html>_ for more details.

Also, maybe put this at the top to increase chances of people noticing this? (if we do want people filing bugs about it, it'd help to increase the chances that they do).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestions. Applied. (Just change should to can. Since I feel should read like users can get this benefit directly. But it is not the case. They need helps from build systems).

And I put this into the What's New in Clang |release|? section, which is highest except the Potentially Breaking Change section.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestions. Applied. (Just change should to can. Since I feel should read like users can get this benefit directly. But it is not the case. They need helps from build systems).

Yeah, makse sense, "can" fits better here, we also want some follow up from the build system vendors.

And I put this into the What's New in Clang |release|? section, which is highest except the Potentially Breaking Change section.

This seems very reasonable, I am not sure this is a "potentially breaking change" even. It does change the BMI, but that's expected with every new version of the compiler.

mode for modules. See `StandardCPlusPlusModules <StandardCPlusPlusModules.html>`_ for
details.

C++23 Feature Support
^^^^^^^^^^^^^^^^^^^^^

Expand Down
128 changes: 128 additions & 0 deletions clang/docs/StandardCPlusPlusModules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,134 @@ in the future. The expected roadmap for Reduced BMIs as of Clang 19.x is:
comes, the term BMI will refer to the Reduced BMI and the Full BMI will only
be meaningful to build systems which elect to support two-phase compilation.

Experimental No Transitive Change
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it actually experimental if it's the default and there is no way to disable it?
I suggest saying "initial", would it better to capture what we intend to do (I suspect we want to keep improving this and want feedback where it falls short)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to find a better term than "no transitive change" -- this doesn't work well as a noun phrase in English, so we say awkward things like "...the no transitive change feature can be more powerful if ..." I think what bothers me about is the "no" and "change" part -- "change" is a verb, so it comes out sounding verb-like and we end up tacking on "feature" to try to make it a noun again.

That said, I don't have a significantly better phrase currently. :-)

Copy link
Contributor

@h-vetinari h-vetinari Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • "no-cascade" feature?
  • "isolated BMI" (not sure if this is a great idea given how there's already full/reduced BMI)
  • "BMI isolation"
  • + variations with synonyms for isolated/secluded/confined/...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it actually experimental if it's the default and there is no way to disable it?

This experiment can only be performed with the build systems. There shouldn't be any risk if the build systems implemented transitive changes model already. In another word, what I did actually provides an opportunity to the build systems to implement this feature. So ideally, the users should be able to disable it in the build system level.

I suggest saying "initial", would it better to capture what we intend to do (I suspect we want to keep improving this and want feedback where it falls short)?

Yes that we want to keep improving this and want feedback where it falls short. I used experimental since I want to mention this feature may not be stable. It might be problematic that some recompilations got skipped but they shouldn't. So that users should go back to traditional mode in such cases. Then I feel experimental sounds good for such situations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to find a better term than "no transitive change" -- this doesn't work well as a noun phrase in English, so we say awkward things like "...the no transitive change feature can be more powerful if ..." I think what bothers me about is the "no" and "change" part -- "change" is a verb, so it comes out sounding verb-like and we end up tacking on "feature" to try to make it a noun again.

While I respect the feelings from native speakers, Google told me that change can be a noun too. For the no part, may be we can use the term less? So that the feature name become to less transitive change, which may be more precise since there should always some changes are transitive.

"no-cascade" feature?

I didn't know this word cascade before. I think it may be appropriate after I searched it. I can replace with it if we're happy with that.

"isolated BMI"

isolated may not be accurate. Since the BMI still depends on other BMIs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slightly cleaner formulation might be "non-cascading X" or perhaps "non-percolating Y".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On "experimental": thanks for clarifying that you meant this as "experimental in the build system".
I am afraid that the current wording actually does the opposite and tells that this the compiler support is "experimental" (aka "unstable", aka "don't touch this until it stabilizes").

On "no transitive change": I'm also not a native speaker, but this did sound weird to me as well. So +1 to changing this.
"Better isolation of BMI" would be my preferred option. "Better" communicates that the isolation is imperfect (in shades of gray rather than black and white), so I wouldn't expect that to cause any confusion. +

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the name: Update with non-cascading change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On "experimental": thanks for clarifying that you meant this as "experimental in the build system". I am afraid that the current wording actually does the opposite and tells that this the compiler support is "experimental" (aka "unstable", aka "don't touch this until it stabilizes").

Yeah, and I like to call it as experimental with the build system. Since although the users can only get it from build systems, the core abilities are provided by the compiler. Also when bug reports comes, I believe most of them need to solve in the compiler side. But this doesn't matter though : )

---------------------------------

Starting with clang19.x, we introduced an experimental feature: the non-transitive
change for modules, aimed at reducing unnecessary recompilations. For example,

.. code-block:: c++

// m-partA.cppm
export module m:partA;

// m-partB.cppm
export module m:partB;
export int getB() { return 44; }

// m.cppm
export module m;
export import :partA;
export import :partB;

// useBOnly.cppm
export module useBOnly;
import m;
export int B() {
return getB();
}

// Use.cc
import useBOnly;
int get() {
return B();
}

Now let's compile the project (For brevity, some commands are omitted.):

.. code-block:: console

$ clang++ -std=c++20 m-partA.cppm --precompile -o m-partA.pcm
$ clang++ -std=c++20 m-partB.cppm --precompile -o m-partB.pcm
$ clang++ -std=c++20 m.cppm --precompile -o m.pcm -fprebuilt-module-path=.
$ clang++ -std=c++20 useBOnly.cppm --precompile -o useBOnly.pcm -fprebuilt-module-path=.
$ md5sum useBOnly.pcm
07656bf4a6908626795729295f9608da useBOnly.pcm

then let's change the interface of ``m-partA.cppm`` to:

.. code-block:: c++

// m-partA.v1.cppm
export module m:partA;
export int getA() { return 43; }

Let's compile the BMI for `useBOnly` again:

.. code-block:: console

$ clang++ -std=c++20 m-partA.cppm --precompile -o m-partA.pcm
$ clang++ -std=c++20 m-partB.cppm --precompile -o m-partB.pcm
$ clang++ -std=c++20 m.cppm --precompile -o m.pcm -fprebuilt-module-path=.
$ clang++ -std=c++20 useBOnly.cppm --precompile -o useBOnly.pcm -fprebuilt-module-path=.
$ md5sum useBOnly.pcm
07656bf4a6908626795729295f9608da useBOnly.pcm

We observed that the contents of useBOnly.pcm remain unchanged.
Consequently, if the build system bases recompilation decisions on directly imported modules only,
it becomes possible to skip the recompilation of Use.cc.
It should be fine because the altered interfaces do not affect Use.cc in any way.
This concept is called as no transitive changes.

When clang generates a BMI, it records the hash values of all potentially contributory BMIs
into the currently written BMI. This ensures that build systems are not required to consider
transitively imported modules when deciding on recompilations.

The definition for potential contributory BMIs is implementation defined. We don't intend to
display detailed rules for users. The contract is:

1. It is a severe bug if a BMI remains unchanged erroneously following an observable change
that affects its users.
2. It is an potential improvement opportunity if a BMI changes after an unobservable change
happens.

We suggest build systems to support this feature as a configurable option for a long time.
So that users can go back to the transitive change mode safely at any time.

Interactions with Reduced BMI
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

With reduced BMI, the no transitive change feature can be more powerful if the change
can be reduced. For example,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not certain what "if the change can be reduced" is trying to say. Is it trying to say that removing code will have less transitive impacts on other BMIs than adding code?

Copy link
Member Author

@ChuanqiXu9 ChuanqiXu9 Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example,

// B.cppm
export module B;
import A;
export int b() { return a(); }

If reduced BMI is not enabled, we will record a() part in the BMI of B, then we will think module A contributes to module B. Then the BMI of B should change if module A changes like the example shows.

However, if reduced BMI enabled, we won't record the function body for int b(), then module A doesn't contributes to module B. So that we can avoid the changes for B if module A changes like the example shows.

Maybe it will be better to remove the if the change can be reduced part. I'll add a sentence to describe the above idea to make it clear.


.. code-block:: c++

// A.cppm
export module A;
export int a() { return 44; }

// B.cppm
export module B;
import A;
export int b() { return a(); }

.. code-block:: console

$ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm -fexperimental-modules-reduced-bmi -o A.o
$ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
$md5sum B.pcm
6c2bd452ca32ab418bf35cd141b060b9 B.pcm

And let's change the implementation for ``A.cppm`` into:

.. code-block:: c++

export module A;
int a_impl() { return 99; }
export int a() { return a_impl(); }

and recompile the example:

.. code-block:: console

$ clang++ -std=c++20 A.cppm -c -fmodule-output=A.pcm -fexperimental-modules-reduced-bmi -o A.o
$ clang++ -std=c++20 B.cppm -c -fmodule-output=B.pcm -fexperimental-modules-reduced-bmi -o B.o -fmodule-file=A=A.pcm
$md5sum B.pcm
6c2bd452ca32ab418bf35cd141b060b9 B.pcm

We should find the contents of ``B.pcm`` keeps the same. In such case, the build system is
allowed to skip recompilations of TUs which solely and directly dependent on module B.

Performance Tips
----------------

Expand Down