ChuanqiXu9 wrote:
I've relanded this in
https://github.com/llvm/llvm-project/commit/947b06282324db8fe2784c4054af9de493a876af.
Let's see what happens.
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
ChuanqiXu9 wrote:
thanks, it is pretty helpful.
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rorth wrote:
This is certainly a case of unaligned access. In a local build, I've run the
first failing `clang` invocation under `truss` (the Solaris syscall tracer).
For
```
/var/llvm/dist-sparcv9-release-stage2-A-flang-clang18/tools/clang/stage2-bins/bin/clang
-cc1 -internal-isystem
ChuanqiXu9 wrote:
Oh, maybe I found the reason. It is because my patch breaks the alignments of
`DeclOffset`:
https://github.com/llvm/llvm-project/blob/8d28e5861f8b117a547850ffbb9a332aa6e91459/clang/include/clang/Serialization/ASTBitCodes.h#L237-L240
then it explains why it work well in some
ChuanqiXu9 wrote:
> > I'll revert this. Due to I can't reproduce this. When the bot gets stable,
> > please tell if it is the real problem.
>
> You can reproduce this: the [GCC compile farm](https://portal.cfarm.net/)
> does have a Solaris/sparcv9 system (`cfarm215`) which is perfectly
rorth wrote:
> I'll revert this. Due to I can't reproduce this. When the bot gets stable,
> please tell if it is the real problem.
You can reproduce this: the [GCC compile farm](https://portal.cfarm.net/) does
have a Solaris/sparcv9 system (`cfarm215`) which is perfectly equipped to run
LLVM
ChuanqiXu9 wrote:
> I strongly suspect that this patch badly broke the [Solaris/sparcv9
> buildbot](https://lab.llvm.org/buildbot/#/builders/72/builds/4046): it
> introduced more than 1000 failures.
>
> Please fix or revert.
I'll revert this. Due to I can't reproduce this. When the bot gets
rorth wrote:
I strongly suspect that this patch badly broke the [Solaris/sparcv9
buildbot](https://lab.llvm.org/buildbot/#/builders/72/builds/4046): it
introduced more than 1000 failures.
Please fix or revert.
https://github.com/llvm/llvm-project/pull/86912
https://github.com/ChuanqiXu9 closed
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Bigcheese approved this pull request.
I did some testing today and this change seems fine. For scanning modules I
actually saw some get smaller with your change.
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits
ChuanqiXu9 wrote:
> The changes LGTM, don't want to block this on my remaining nits.
Thanks for reviewing this.
>
> I believe @Bigcheese wanted to test test impact on PCM size on our side
> before this lands.
I've rebased this with main. I'll wait for the results from @Bigcheese
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From 2c20a6200fb2790b3a891ffc8c43682c113c7e8a Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH] [Modules] No transitive source location change
---
https://github.com/jansvoboda11 approved this pull request.
The changes LGTM, don't want to block this on my remaining nits.
I believe @Bigcheese wanted to test test impact on PCM size on our side before
this lands.
https://github.com/llvm/llvm-project/pull/86912
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2221,33 +,45 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
ChuanqiXu9 wrote:
@jansvoboda11 @Bigcheese ping
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From ddb4074b0460daf7b42531ec62e97347b3f2e14d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH 1/4] [Modules] No transitive source location change
---
@@ -4082,14 +4069,14 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile )
const {
: ModuleMgr.lookupByFileName(Name));
if (!OM) {
std::string Msg =
- "SourceLocation remap refers to unknown module, cannot find ";
+ "cannot
@@ -2221,33 +,45 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -5574,10 +5577,34 @@ void ASTWriter::AddFileID(FileID FID, RecordDataImpl
) {
Record.push_back(getAdjustedFileID(FID).getOpaqueValue());
}
+SourceLocationEncoding::RawLocEncoding
+ASTWriter::getRawSourceLocationEncoding(SourceLocation Loc, LocSeq *Seq) {
+ unsigned
@@ -4082,14 +4069,14 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile )
const {
: ModuleMgr.lookupByFileName(Name));
if (!OM) {
std::string Msg =
- "SourceLocation remap refers to unknown module, cannot find ";
+ "cannot
@@ -2221,33 +,45 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
ChuanqiXu9 wrote:
Fix conflicts and rebase with main.
@Bigcheese @jansvoboda11 ping~
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From 8e07cbdd0f348484d532d7827e4b4a7888e70978 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH 1/3] [Modules] No transitive source location change
---
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,33 +2221,40 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
ChuanqiXu9 wrote:
> I have a few minor comments on the patch. I want to do some additional perf
> testing on module scanning perf because I'm a bit concerned about the cost of
> `ASTWriter::getRawSourceLocationEncoding`.
What's the perf results about scanning?
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -4078,8 +4065,8 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile ) const {
return;
ChuanqiXu9 wrote:
Done.
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -696,7 +696,7 @@ class ASTReader
/// Mapping from global submodule IDs to the module file in which the
/// submodule resides along with the offset that should be added to the
/// global submodule ID to produce a local ID.
- GlobalSubmoduleMapType GlobalSubmoduleMap;
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -5574,10 +5577,34 @@ void ASTWriter::AddFileID(FileID FID, RecordDataImpl
) {
Record.push_back(getAdjustedFileID(FID).getOpaqueValue());
}
+SourceLocationEncoding::RawLocEncoding
+ASTWriter::getRawSourceLocationEncoding(SourceLocation Loc, LocSeq *Seq) {
+ unsigned
@@ -942,6 +942,12 @@ class ASTReader
/// Sema tracks these to emit deferred diags.
llvm::SmallSetVector DeclsToCheckForDeferredDiags;
+ /// The module files imported by different module files. Indirectly imported
+ /// module files are included too. The information
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From 8e07cbdd0f348484d532d7827e4b4a7888e70978 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH 1/2] [Modules] No transitive source location change
---
ChuanqiXu9 wrote:
> Could you share the rest of the "no transitive change" patches, or at least
> their outline? I'm curious what else goes into making that work.
Oh, I haven't started them. The patch in my mind includes two at least:
(1) Add a new block recording the semantical hash value of
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
@@ -4078,8 +4065,8 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile ) const {
return;
Bigcheese wrote:
There's no more SourceLocation remap so I believe this error message is wrong
now.
https://github.com/llvm/llvm-project/pull/86912
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
https://github.com/Bigcheese edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Bigcheese commented:
I have a few minor comments on the patch. I want to do some additional perf
testing on module scanning perf because I'm a bit concerned about the cost of
`ASTWriter::getRawSourceLocationEncoding`.
https://github.com/llvm/llvm-project/pull/86912
jansvoboda11 wrote:
I left a couple of initial comments. With the 64-bit `SourceLocation` aspect of
this change solved and with just 2% size increase, I think this approach makes
sense. Could you share the rest of the "no transitive change" patches, or at
least their outline? I'm curious what
@@ -696,7 +696,7 @@ class ASTReader
/// Mapping from global submodule IDs to the module file in which the
/// submodule resides along with the offset that should be added to the
/// global submodule ID to produce a local ID.
- GlobalSubmoduleMapType GlobalSubmoduleMap;
@@ -2220,40 +2227,47 @@ class ASTReader
return Sema::AlignPackInfo::getFromRawEncoding(Raw);
}
+ using RawLocEncoding = SourceLocationEncoding::RawLocEncoding;
+
/// Read a source location from raw form and return it in its
/// originating module file's source
@@ -5574,10 +5577,34 @@ void ASTWriter::AddFileID(FileID FID, RecordDataImpl
) {
Record.push_back(getAdjustedFileID(FID).getOpaqueValue());
}
+SourceLocationEncoding::RawLocEncoding
+ASTWriter::getRawSourceLocationEncoding(SourceLocation Loc, LocSeq *Seq) {
+ unsigned
@@ -942,6 +942,12 @@ class ASTReader
/// Sema tracks these to emit deferred diags.
llvm::SmallSetVector DeclsToCheckForDeferredDiags;
+ /// The module files imported by different module files. Indirectly imported
+ /// module files are included too. The information
@@ -149,14 +157,44 @@ class SourceLocationSequence::State {
operator SourceLocationSequence *() { return }
};
-inline uint64_t SourceLocationEncoding::encode(SourceLocation Loc,
- SourceLocationSequence *Seq) {
- return Seq ?
https://github.com/Bigcheese commented:
I think the general approach makes sense. I'll take a closer look at the
specific changes.
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ChuanqiXu9 wrote:
Rebase with main and enabling the sequence optimization when the module file
index is 0. Now the size of the PCMs goes to 204M (originally it is 200M). It
looks better.
@jansvoboda11 @Bigcheese ping
https://github.com/llvm/llvm-project/pull/86912
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From 8e07cbdd0f348484d532d7827e4b4a7888e70978 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH] [Modules] No transitive source location change
---
statham-arm wrote:
> Let's see if @statham-arm (who introduced the `SourceLocation::[U]IntTy`
> typedefs) wants to weight in here.
I'm afraid my knowledge of C++ modules is very close to zero. They were
mentioned in a training course I did last year, but not in much detail.
On 64-bit
ChuanqiXu9 wrote:
> > If Clang is configured with 64 bit `SourceLocation`, we can't use 96 bits
> > for serialization. We can at most use 64 bits for a slot. In that case, we
> > can only assume the offset of source location **in its own module** (not
> > the global offset!) is not large than
jansvoboda11 wrote:
> If Clang is configured with 64 bit `SourceLocation`, we can't use 96 bits for
> serialization. We can at most use 64 bits for a slot. In that case, we can
> only assume the offset of source location **in its own module** (not the
> global offset!) is not large than 2^32.
ChuanqiXu9 wrote:
> > Yes, I explained this in Some low level details section. The size of source
> > location won't be affected. Since the size of source location is unsigned
> > (practically, it is 32 bits in most platforms). And we use uint64_t as a
> > unit in the serializer. So there are
jansvoboda11 wrote:
> Yes, I explained this in Some low level details section. The size of source
> location won't be affected. Since the size of source location is unsigned
> (practically, it is 32 bits in most platforms). And we use uint64_t as a unit
> in the serializer. So there are 32
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ChuanqiXu9 wrote:
> By default, `SourceLocation` is 32 bits. One bit is used to distinguish macro
> expansions. Looking at libc++'s module map, it contains 999 top-level modules
> at this moment. That's 10 bits just to be able to import the (entire)
> standard library. That leaves 21 bits,
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
jansvoboda11 wrote:
By default, `SourceLocation` is 32 bits. One bit is used to distinguish macro
expansions. Looking at libc++'s module map, it contains 999 top-level modules
at this moment. That's 10 bits just to be able to import the (entire) standard
library. That leaves 21 bits,
jyknight wrote:
+1 on the high-level plan. Switching from a linear offset to a
{local-module-index, offset-within-module} pair sounds great!
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ChuanqiXu9 wrote:
BTW, after this patch, with reduced BMI
(https://github.com/llvm/llvm-project/pull/85050), we can already do something
more interesting than reformating:
```
//--- A.cppm
export module A;
int funcA0();
int funcA1();
export int funcA() {
return funcA0();
}
//---
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Chuanqi Xu (ChuanqiXu9)
Changes
This is part of "no transitive change" patch series, "no transitive source
location change". I talked this with @Bigcheese in the tokyo's WG21
meeting.
The idea comes from @jyknight posted on LLVM
https://github.com/ChuanqiXu9 ready_for_review
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/86912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/86912
>From 8d4e349710f4ca84a7ad2dd8aa71afa50b730dbb Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Mon, 18 Mar 2024 08:36:55 +0800
Subject: [PATCH] [Modules] No transitive source location change
---
86 matches
Mail list logo
