[llvm] [clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

ktf wrote: I was carried away by the heavy ion run and other priorities. i can try to have a look again after Christmas. Feel free to close if you prefer. Ciao, Giulio On Fri, Dec 22 2023 at 9:32 AM, Vassil Vassilev ***@***.***> wrote: > @ktf what is the fate of this

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/9] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/8] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -489,6 +490,8 @@ class SLocEntry { bool isExpansion() const { return IsExpansion; } bool isFile() const { return !isExpansion(); } + bool isLoaded() const { return Loaded; } + void setLoaded(bool Value) { Loaded = Value; } vgvassilev wrote:

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -474,9 +474,10 @@ static_assert(sizeof(FileInfo) <= sizeof(ExpansionInfo), /// SourceManager keeps an array of these objects, and they are uniquely /// identified by the FileID datatype. class SLocEntry { - static constexpr int OffsetBits = 8 *

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/kuhar commented: The ADT change looks good to me, I'm not familiar with the clang code though. https://github.com/llvm/llvm-project/pull/67960 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/7] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

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

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -180,6 +180,20 @@ TEST(PagedVectorTest, FillNonTrivialConstructor) { EXPECT_EQ(std::distance(V.materialized_begin(), V.materialized_end()), 10LL); } +// Test that isMaterialized returns true for all the elements +// of the page, not only the one that was accessed.

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

ktf wrote: IIRC I saw a ~0.3 MB reduction in memory usage in my usual test. I do not see any performance change, although that is not particularly the focus of my investigation here, so I cannot exclude it (hopefully for the better). https://github.com/llvm/llvm-project/pull/67960

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/6] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -489,6 +490,8 @@ class SLocEntry { bool isExpansion() const { return IsExpansion; } bool isFile() const { return !isExpansion(); } + [[nodiscard]] bool isLoaded() const { return Loaded; } ktf wrote: No, I guess not. It's just my clang-tidy

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/vgvassilev commented: That's indeed what I was proposing. Do we observe some performance benefits from this PR on the downstream codebase? https://github.com/llvm/llvm-project/pull/67960 ___ cfe-commits mailing list

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -489,6 +490,8 @@ class SLocEntry { bool isExpansion() const { return IsExpansion; } bool isFile() const { return !isExpansion(); } + [[nodiscard]] bool isLoaded() const { return Loaded; } vgvassilev wrote: Do we need to add `[[nodiscard]]` to the

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

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

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -103,6 +103,14 @@ template class PagedVector { /// Return the size of the vector. [[nodiscard]] size_t size() const { return Size; } + /// Return true if the element at `Index` belongs to a page which was already + /// materialized, i.e., had at least one element

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/5] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -103,6 +103,14 @@ template class PagedVector { /// Return the size of the vector. [[nodiscard]] size_t size() const { return Size; } + /// Return true if the element at `Index` belongs to a page which was already + /// materialized, i.e., had at least one element

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/4] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -103,6 +103,14 @@ template class PagedVector { /// Return the size of the vector. [[nodiscard]] size_t size() const { return Size; } + /// @return true in case the element at index @a Index belongs to a page which + /// was already materialised.

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/3] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -103,6 +103,12 @@ template class PagedVector { /// Return the size of the vector. [[nodiscard]] size_t size() const { return Size; } + [[nodiscard]] bool isMaterialized(size_t Index) const { ktf wrote: ```suggestion /// @return true in case the

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67960 >From 9fde224de6baa5b1fb3713d810ce835d4456b457 Mon Sep 17 00:00:00 2001 From: Giulio Eulisse <10544+...@users.noreply.github.com> Date: Fri, 29 Sep 2023 08:37:57 +0200 Subject: [PATCH 1/2] Avoid need for

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -103,6 +103,12 @@ template class PagedVector { /// Return the size of the vector. [[nodiscard]] size_t size() const { return Size; } + [[nodiscard]] bool isMaterialized(size_t Index) const { kuhar wrote: Could you add a documentation comment above?

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

@@ -103,6 +103,12 @@ template class PagedVector { /// Return the size of the vector. [[nodiscard]] size_t size() const { return Size; } + [[nodiscard]] bool isMaterialized(size_t Index) const { +assert(Index < Size); +assert(Index / PageSize <

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

ktf wrote: @vgvassilev IIUC, this is what you were proposing in some comment in https://github.com/llvm/llvm-project/pull/66430. https://github.com/llvm/llvm-project/pull/67960 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

llvmbot wrote: @llvm/pr-subscribers-clang Changes The BitVector is currently used to keep track of which entries of LoadedSLocEntryTable have been loaded. Such information can be stored in the SLocEntry itself. Moreover, thanks to the fact that LoadedSLocEntryTable is now a PagedVector,

[clang] Avoid need for SLocEntryLoaded BitVector (PR #67960)

https://github.com/ktf created https://github.com/llvm/llvm-project/pull/67960 The BitVector is currently used to keep track of which entries of LoadedSLocEntryTable have been loaded. Such information can be stored in the SLocEntry itself. Moreover, thanks to the fact that LoadedSLocEntryTable