[clang] [llvm] [Serialization] Use stable hash functions (PR #96136)

ChuanqiXu9 wrote: This looks generally good. > Some check-clang-modules tests still fail (@ChuanqiXu9) What does this mean? Do you mean there are some failures after this patch? What are they? Or are these tests failing before this patch? https://github.com/llvm/llvm-project/pull/96136

[clang] [Serialization] Storing DeclID separately (PR #95897)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/95897 >From b6d1326fdee4f31c6f6e32783c690b7ae2a4dedb Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 18 Jun 2024 11:28:03 +0800 Subject: [PATCH] Draft --- clang/include/clang/AST/DeclID.h | 2

[clang] 8af8602 - [NFC] [Serialization] Unify how LocalDeclID can be created

Author: Chuanqi Xu Date: 2024-06-19T15:18:01+08:00 New Revision: 8af86025af2456c70c84aec309cca9a069124671 URL: https://github.com/llvm/llvm-project/commit/8af86025af2456c70c84aec309cca9a069124671 DIFF: https://github.com/llvm/llvm-project/commit/8af86025af2456c70c84aec309cca9a069124671.diff

[clang] [llvm] [Clang][Coroutines] Introducing the `[[clang::coro_inplace_task]]` attribute (PR #94693)

ChuanqiXu9 wrote: > > As I said, I prefer to do this in Parser or Sema if possible. > > Most of the logic is already in Sema in this version. This piece is at the > very end of funnel in FE. Hence I am asking about what mechanisms we can use > to obtain the `CallInst` or `InvokeInst` based on

[clang] [Serialization] Storing DeclID separately (PR #95897)

https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/95897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Serialization] Storing DeclID separately (PR #95897)

https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/95897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Serialization] Storing DeclID separately (PR #95897)

https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/95897 In https://github.com/llvm/llvm-project/pull/92083, there are some unaddressed question about the increased size of PCMs. And I prepared this draft to try to address this. This is patch is not ready for

[clang] [Serialization] Use specialized decl hash function for GlobalDeclID (PR #95730)

@@ -230,7 +230,11 @@ template <> struct DenseMapInfo { } static unsigned getHashValue(const GlobalDeclID ) { -return DenseMapInfo::getHashValue(Key.get()); +// Our default hash algorithm for 64 bits integer may not be very good. +// In GlobalDeclID's case, it

[clang] [Serialization] Use specialized decl hash function for GlobalDeclID (PR #95730)

@@ -230,7 +230,11 @@ template <> struct DenseMapInfo { } static unsigned getHashValue(const GlobalDeclID ) { -return DenseMapInfo::getHashValue(Key.get()); +// Our default hash algorithm for 64 bits integer may not be very good. ChuanqiXu9 wrote:

[clang] [Serialization] Use specialized decl hash function for GlobalDeclID (PR #95730)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/95730 >From 50923aa33f09b2530cfe492a53f70296f9ce9107 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Mon, 17 Jun 2024 11:32:35 +0800 Subject: [PATCH 1/2] [Serialization] Use specialized decl hash function for

[clang] [Serialization] Don't read all declaration id eagerly when merge the tables (PR #95506)

ChuanqiXu9 wrote: > @ChuanqiXu9 are you still planning to chase this given that fixing the > hashing function would fix performance for the aforementioned patch? > > The trade-off we are making here is hard to assess without benchmarks that > show how much latency we win and how much more

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: > Oh, I didn't realize you were eager to land this in Clang 19, thanks for > sharing that. In that case, I think a specialiazed hash function for > `GlobalDeclID` is indeed the way to go. > > I was also worried a little there are other performance implications of this >

[clang] [Serialization] No transitive identifier change (PR #92085)

ChuanqiXu9 wrote: Thanks. Will do when landing. https://github.com/llvm/llvm-project/pull/92085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [Clang][Coroutines] Improve CoroElide with [[clang::coro_structured_concurrency]] attribute for C++ (PR #94693)

ChuanqiXu9 wrote: > Thanks for the feedback. This patch is the first iteration to model this idea > as quickly as I can. In general, I agree with your comments. > > > * I feel the name containing `concurrency` is not proper > > The name is bikesheddable as always. I was also thinking around

[clang] [llvm] [Clang][Coroutines] Improve CoroElide with [[clang::coro_structured_concurrency]] attribute for C++ (PR #94693)

ChuanqiXu9 wrote: > Thanks for the feedback. This patch is the first iteration to model this idea > as quickly as I can. In general, I agree with your comments. > > > * I feel the name containing `concurrency` is not proper > > The name is bikesheddable as always. I was also thinking around

[clang] [Serialization] Use specialized decl hash function for GlobalDeclID (PR #95730)

https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/95730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Serialization] Use specialized decl hash function for GlobalDeclID (PR #95730)

https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/95730 See the comment: https://github.com/llvm/llvm-project/pull/92083#issuecomment-2168121729 After the patch, https://github.com/llvm/llvm-project/pull/92083, the lower 32 bits of DeclID can be the same

[clang] [llvm] [Clang][Coroutines] Improve CoroElide with [[clang::coro_structured_concurrency]] attribute for C++ (PR #94693)

ChuanqiXu9 wrote: Then we can find that the frontend part is much more complex and harder than the middle end part. So maybe we can introduce the middle end part first and introduce a not so powerful attribute but introducing a function and statement level attribute `must_elide` in the

[clang] [llvm] [Clang][Coroutines] Improve CoroElide with [[clang::coro_structured_concurrency]] attribute for C++ (PR #94693)

ChuanqiXu9 wrote: Thanks for the patch. I like it in the very high level. I did a quick scanning over the PR and here is some comments: - I feel the name containing `concurrency` is not proper. I don't feel it relates concurrency in any level. - Every time we add or change IR related to

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/75912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -6853,6 +6853,7 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) { if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never) DI->completeUnusedClass(*CRD); } + ChuanqiXu9 wrote: Done

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912 >From 4b595ee5b7a5fefb1e9ec0a152bce587ba463b4b Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/8] [C++20] [Modules] [Itanium ABI] Generate the vtable in the

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -318,6 +318,9 @@ namespace { if (Diags.hasUnrecoverableErrorOccurred()) return; + if (RD->shouldEmitInExternalSource()) ChuanqiXu9 wrote: Thanks. It makes sense. https://github.com/llvm/llvm-project/pull/75912

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: > I got to the bottom of it. The problem is that our hash function for 64 bit > ints is [not very > good](https://github.com/llvm/llvm-project/blob/d5297b72aa32ad3a69563a1fcc61294282f0b379/llvm/include/llvm/ADT/DenseMapInfo.h#L140). > > It will have a lot of collision when

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,199 @@ +//===-- ProjectModules.h -*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,347 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,347 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,347 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,364 @@ +//===--- PrerequisiteModulesTests.cpp ---*- C++ +//-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +//

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,364 @@ +//===--- PrerequisiteModulesTests.cpp ---*- C++ +//-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +//

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -213,10 +213,10 @@ ClangdServer::Options::operator TUScheduler::Options() const { ClangdServer::ClangdServer(const GlobalCompilationDatabase , const ThreadsafeFS , const Options , - Callbacks *Callbacks) +

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,26 @@ +//=== ScanningProjectModules.h ---*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -710,6 +710,7 @@ class ASTWorker { TUScheduler::ASTCache TUScheduler::HeaderIncluderCache const bool RunSync; + ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,347 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,81 @@ +//=== ModuleDependencyScanner.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -27,6 +27,9 @@ #include "Diagnostics.h" #include "FS.h" #include "Headers.h" + ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

ChuanqiXu9 wrote: Oh, maybe I was misreading. It looks doable if we only want to avoid DirectoryBasedGlobalCompilationDatabase. Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,370 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,347 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,347 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,370 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -112,6 +114,9 @@ class ClangdServer { /// This throttler controls which preambles may be built at a given time. clangd::PreambleThrottler *PreambleThrottler = nullptr; +/// Enable experimental support for modules. ChuanqiXu9 wrote: I add

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: I sent https://github.com/llvm/llvm-project/pull/95506. It is a independent patch which may mitigate the issue you met. @alexfh you can try this when you have time. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits

[clang] [Serialization] Don't read all declaration id eagerly when merge the tables (PR #95506)

https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/95506 See the post commit message in https://github.com/llvm/llvm-project/pull/92083 for rationale. Previously, when we merge the lookup tables, we will read the tables completely to get the data. But the above

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: Thanks for the profiling data. It narrows the scope a lot. But it makes me confusing too. Since the scope is pretty narrow, we can do an analysis here:

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -318,6 +318,9 @@ namespace { if (Diags.hasUnrecoverableErrorOccurred()) return; + if (RD->shouldEmitInExternalSource()) ChuanqiXu9 wrote: Done. I don't mind doing it really. But what I confuse is, it looks there are a lot of codes in

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912 >From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/7] [C++20] [Modules] [Itanium ABI] Generate the vtable in the

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: > The 10x increase in the time spent reading modules does look surprising and I > would need to check if it's related to PCMs becoming larger or is localized > to that particular compile. (Even if on a single example). It should not related to the size of the PCMs otherwise

[clang] [Serialization] Use 32 bit aligned decl id instead of unaligned decl id (PR #95348)

ChuanqiXu9 wrote: @alexfh Could you try to test this? And if this doesn't mitigate it, it will be helpful to provide the hotspot. https://github.com/llvm/llvm-project/pull/95348 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: I sent https://github.com/llvm/llvm-project/pull/95348 for aligned related change. For size increase, the reason and the possible solution are clear. We should store the declaration ID as two slots instead of one in the serialized format so that we can utilize VBR6 format

[clang] [Serialization] Use 32 bit aligned decl id instead of unaligned decl id (PR #95348)

https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/95348 See the post commit message in https://github.com/llvm/llvm-project/pull/92083. I suspect the compile time regression in AArch64 is related to alignments. I am not sure if this is the problem since I can't

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -318,6 +318,9 @@ namespace { if (Diags.hasUnrecoverableErrorOccurred()) return; + if (RD->shouldEmitInExternalSource()) ChuanqiXu9 wrote: Yes, I think it is not bad to put the check `RD->shouldEmitInExternalSource()` into

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -318,6 +318,9 @@ namespace { if (Diags.hasUnrecoverableErrorOccurred()) return; + if (RD->shouldEmitInExternalSource()) ChuanqiXu9 wrote: I use `isInCurrentModuleUnit` instead since I feel the semantics are more clear.

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912 >From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/6] [C++20] [Modules] [Itanium ABI] Generate the vtable in the

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: Oh, the time regression is surprising to me. And I observed that @alexfh only reports (compile performance) problems on AArch64 but @bgra8 reports (size increase problems) on both ARM64 and X86_64. @alexfh is it true? So I **guess** the problem may come from the unaligned

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

https://github.com/ChuanqiXu9 approved this pull request. LGTM. Thanks. https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -6853,6 +6853,13 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) { if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never) DI->completeUnusedClass(*CRD); } +// If we're emitting a dynamic class from the importable module we're +//

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -1227,7 +1241,8 @@ void CodeGenModule::EmitDeferredVTables() { #endif for (const CXXRecordDecl *RD : DeferredVTables) -if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD)) +if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD) && +

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912 >From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/5] [C++20] [Modules] [Itanium ABI] Generate the vtable in the

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

ChuanqiXu9 wrote: I feel better to have a regression test for this if possible. I am just worrying someone someday meet the problem unconsciously. https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

ChuanqiXu9 wrote: > It still fixes the original bug report though. Yeah, this is why it is good. > > Otherwise adding a test which directly tests all observable effects of the > profiling fix would be a bit arbitrary, as we don't have any unit tests for > the gazillion ways this could go

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/95202 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

@@ -1770,6 +1770,10 @@ class ASTContext : public RefCountedBase { QualType getDeducedTemplateSpecializationType(TemplateName Template, QualType DeducedType, bool IsDependent)

[clang] [clang] fix broken canonicalization of DeducedTemplateSpecializationType (PR #95202)

https://github.com/ChuanqiXu9 commented: Oh, thanks. This looks generally good. And it looks like true that I abused the Profile components. Since this commit aimed to fix a regression, it will be better to have a regression test. WDYT? https://github.com/llvm/llvm-project/pull/95202

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: Oh, this is surprising. Are arm CPUs 32 bit? And how much do these tests get slowed down? https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -1830,6 +1830,9 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + if (RD->shouldEmitInExternalSource()) +return; ChuanqiXu9 wrote: Yes and agreed. I just don't have an idea for how do that.

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -6853,6 +6853,13 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) { if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never) DI->completeUnusedClass(*CRD); } +// If we're emitting a dynamic class from the importable module we're +//

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -3219,7 +3219,8 @@ void CodeGenModule::EmitVTablesOpportunistically() { for (const CXXRecordDecl *RD : OpportunisticVTables) { assert(getVTables().isVTableExternal(RD) && "This queue should only contain external vtables"); -if

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912 >From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/4] [C++20] [Modules] [Itanium ABI] Generate the vtable in the

[clang] [Serialization] No transitive identifier change (PR #92085)

ChuanqiXu9 wrote: @jansvoboda11 ping~ I hope we can land the series of the patches in 2 weeks to give more baking times for them https://github.com/llvm/llvm-project/pull/92085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)

https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/94603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: Thanks for testing. I've resent https://github.com/llvm/llvm-project/commit/5a0181f568e56e37df80d0f74eca4775776fa8cd. https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] 5a0181f - [serialization] no transitive decl change (#92083)

Author: Chuanqi Xu Date: 2024-06-07T20:21:55+08:00 New Revision: 5a0181f568e56e37df80d0f74eca4775776fa8cd URL: https://github.com/llvm/llvm-project/commit/5a0181f568e56e37df80d0f74eca4775776fa8cd DIFF: https://github.com/llvm/llvm-project/commit/5a0181f568e56e37df80d0f74eca4775776fa8cd.diff

[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)

ChuanqiXu9 wrote: Thank you very much! https://github.com/llvm/llvm-project/pull/94603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Safe Buffers] Serialize unsafe_buffer_usage pragmas (PR #92031)

@@ -2439,6 +2440,12 @@ void ASTWriter::WritePreprocessor(const Preprocessor , bool IsModule) { Record.clear(); } + // Write the safe buffer opt-out region map in PP + for (SourceLocation : PP.serializeSafeBufferOptOutMap()) +AddSourceLocation(std::move(S),

[clang] [Safe Buffers] Serialize unsafe_buffer_usage pragmas (PR #92031)

https://github.com/ChuanqiXu9 commented: Serialization changes look good to me. I'll leave the formal approve chance to others. https://github.com/llvm/llvm-project/pull/92031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [Safe Buffers] Serialize unsafe_buffer_usage pragmas (PR #92031)

https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/92031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Safe Buffers] Serialize unsafe_buffer_usage pragmas (PR #92031)

@@ -0,0 +1,41 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -emit-module -fmodule-name=safe_buffers_test_base -x c++ %S/Inputs/SafeBuffers/safe_buffers_test.modulemap -std=c++20\ +// RUN: -o %t/safe_buffers_test_base.pcm -Wunsafe-buffer-usage

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -1830,6 +1830,9 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + if (RD->shouldEmitInExternalSource()) +return; ChuanqiXu9 wrote: Got it. If it is primarily for optimizations, I guess it

[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)

ChuanqiXu9 wrote: @DavidSpickett oh, sorry. I didn't expect the refined patch will break 32 bit again. Can you try the new patch? https://github.com/llvm/llvm-project/pull/94603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: Sadly it looks like the (refined) patch still breaks the alignment on 32 bit machine. https://lab.llvm.org/buildbot/#/builders/174/builds/34307 I've reverted the patch and updated https://github.com/llvm/llvm-project/pull/94603 to test it again.

[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/94603 >From 264041a5819f7842ad5883e76b74de64615ddea1 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Thu, 6 Jun 2024 11:51:05 +0800 Subject: [PATCH 1/2] [serialization] no transitive decl change (#92083) Following

[clang] 4f70c5e - Revert "[serialization] no transitive decl change (#92083)"

Author: Chuanqi Xu Date: 2024-06-07T11:29:09+08:00 New Revision: 4f70c5ec4a57e84642fa0772536f120cd9c75edb URL: https://github.com/llvm/llvm-project/commit/4f70c5ec4a57e84642fa0772536f120cd9c75edb DIFF: https://github.com/llvm/llvm-project/commit/4f70c5ec4a57e84642fa0772536f120cd9c75edb.diff

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -1830,6 +1830,9 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + if (RD->shouldEmitInExternalSource()) +return; ChuanqiXu9 wrote: > This check seems like it's at the wrong level; if we don't

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/75912 >From cf8be3c418dde67b74d4a5a4ea98a33f0e2fbd72 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 19 Dec 2023 17:00:59 +0800 Subject: [PATCH 1/3] [C++20] [Modules] [Itanium ABI] Generate the vtable in the

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: Resent https://github.com/llvm/llvm-project/commit/5c104879c1a98eeb845c03e7c45206bd48e88f0c Thanks for testing it ! https://github.com/llvm/llvm-project/pull/92083 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] 5c10487 - [serialization] no transitive decl change (#92083)

Author: Chuanqi Xu Date: 2024-06-07T10:47:53+08:00 New Revision: 5c104879c1a98eeb845c03e7c45206bd48e88f0c URL: https://github.com/llvm/llvm-project/commit/5c104879c1a98eeb845c03e7c45206bd48e88f0c DIFF: https://github.com/llvm/llvm-project/commit/5c104879c1a98eeb845c03e7c45206bd48e88f0c.diff

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -1185,6 +1190,21 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) { TSK == TSK_ExplicitInstantiationDefinition) return false; + // Itanium C++ ABI [5.2.3]: + // Virtual tables for dynamic classes are emitted as follows: + // + // - If the

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -1830,6 +1830,9 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables , if (VTable->hasInitializer()) return; + if (RD->shouldEmitInExternalSource()) +return; ChuanqiXu9 wrote: > Also, even if the vtable is getting emitted somewhere

[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

@@ -6853,6 +6853,13 @@ void CodeGenModule::EmitTopLevelDecl(Decl *D) { if (ES->hasExternalDefinitions(D) == ExternalASTSource::EK_Never) DI->completeUnusedClass(*CRD); } +// If we're emitting a dynamic class from the importable module we're +//

[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)

ChuanqiXu9 wrote: Thanks! https://github.com/llvm/llvm-project/pull/94603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [serialization] no transitive decl change (PR #92083)

ChuanqiXu9 wrote: > > I didn't recognize we may still have 32-bit machines > > I think 32 bit Arm are the last. > > > I'll revert it again. > > Sure, if you post the PR I can test the fix too. I know getting 32 bit > machines can be tricky. I sent

[clang] Testing32 bit for https://github.com/llvm/llvm-project/pull/92083 (PR #94603)

https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/94603 https://github.com/llvm/llvm-project/pull/92083 is failed on 32bit machine but it is not easy to get a 32 bit machine. And thanks for @DavidSpickett would love to test this. I tried to review the code and I

[clang] e285818 - Revert "[serialization] no transitive decl change (#92083)"

Author: Chuanqi Xu Date: 2024-06-06T17:49:59+08:00 New Revision: e2858189bd99e6914dc2f63ab55b053a74b4e58b URL: https://github.com/llvm/llvm-project/commit/e2858189bd99e6914dc2f63ab55b053a74b4e58b DIFF: https://github.com/llvm/llvm-project/commit/e2858189bd99e6914dc2f63ab55b053a74b4e58b.diff

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

ChuanqiXu9 wrote: > Just a note, I am building on Windows with MSVC cl.exe and ninja and get this: > > ``` > C:\Program Files\Microsoft Visual > Studio\2022\Community\VC\Tools\MSVC\14.39.33519\include\memory(3138): error > C2027: use of undefined type 'clang::clangd::ProjectModules' > ``` >

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

ChuanqiXu9 wrote: > I think that needs to happen eventually as well Agreed. > similar to preambles supporting in-memory storage, and it's actually not that > hard, GenerateModuleInterfaceAction already has an overrideable > CreateOutputFile method. I am

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,87 @@ +//===- PrerequisiteModules.h -*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

ChuanqiXu9 wrote: Done https://github.com/llvm/llvm-project/pull/66462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules (PR #66462)

@@ -0,0 +1,370 @@ +//===- ModulesBuilder.cpp *- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier:

<    1   2   3   4   5   6   7   8   9   10   >