[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D144844#4197067 , @dblaikie wrote: > I don't think of this as a performance regression for users though - this > functionality's never really "shipped" so we get to choose what the baseline > is. > > And I think a

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D144844#4195633 , @ChuanqiXu wrote: >> However, "performance" also includes compilation speed in the 'no >> optimisation, debug' case - that is also considered very important. So, >> perhaps, the short-term approach should be

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. I don't think of this as a performance regression for users though - this functionality's never really "shipped" so we get to choose what the baseline is. And I think a reasonable baseline to compare to isn't this implementation we don't think is ideal (because of the

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > However, "performance" also includes compilation speed in the 'no > optimisation, debug' case - that is also considered very important. So, > perhaps, the short-term approach should be (as @dblaikie suggested) to > include the bodies for -O >= 3? I don't think so.

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D144844#4195316 , @ChuanqiXu wrote: > Got your points. Let's postpone this one. > > But I want to emphasize that this patch (and the thin PCM) will decrease the > performance. While LTO can save the regression, LTO is not

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-14 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu planned changes to this revision. ChuanqiXu added a comment. It should be "Plan Changed" instead of "Abandoned". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144844/new/ https://reviews.llvm.org/D144844

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-14 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu abandoned this revision. ChuanqiXu added a comment. Got your points. Let's postpone this one. But I want to emphasize that this patch (and the thin PCM) will decrease the performance. While LTO can save the regression, LTO is not widely used. (ThinLTO can only mitigate this.) I mean

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-14 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D144844#4193727 , @iains wrote: > In D144844#4193568 , @dblaikie > wrote: > >> Seem to recall @iains and others were concerned about the number of modules >> flags - this one I'd be

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D144844#4193568 , @dblaikie wrote: > Seem to recall @iains and others were concerned about the number of modules > flags - this one I'd be inclined to suggest we shouldn't add if possible. If > one way or the other is

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-14 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Seem to recall @iains and others were concerned about the number of modules flags - this one I'd be inclined to suggest we shouldn't add if possible. If one way or the other is generally better - we should, at least initially, dictate that behavior entirely until

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @iains @dblaikie gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144844/new/ https://reviews.llvm.org/D144844 ___ cfe-commits mailing list

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-02-27 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, Bigcheese, dblaikie. ChuanqiXu added a project: clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Close