This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a1386c81de5: [Modules][PCH] Hash input files content
(authored by bruno).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org/D67249
___
bruno added a reviewer: v.g.vassilev.
bruno added a comment.
Ping!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org/D67249
___
cfe-commits mailing list
cfe-commits@list
ributzka added a comment.
xxhash64 is apparently faster than MD5 and SHA1, and produces good quality
hashes. I am not sure about the hash quality of hash_code for this purpose.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://revie
bruno added a comment.
> Did you try xxHash64?
No, and I'm not planning to. I believe `hash_code` is enough here, already has
low overhead on performance and size, and bunch of prior use elsewhere in
LLVM/Clang. If there's a strong reason to do it I wanna know why that's the
case first.
Re
ributzka added a comment.
Did you try xxHash64?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org/D67249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://
bruno updated this revision to Diff 219607.
bruno added a comment.
Update the patch to use two ::Fixed, 32 in abbrev to encode hash.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org/D67249
Files:
clang/include/cl
bruno marked an inline comment as done.
bruno added inline comments.
Comment at: clang/lib/Serialization/ASTWriter.cpp:1767
bool IsTopLevelModuleMap;
+ uint32_t ContentHash[2];
};
ributzka wrote:
> bruno wrote:
> > aprantl wrote:
> > > Why is this not a uint
ributzka added inline comments.
Comment at: clang/lib/Serialization/ASTWriter.cpp:1767
bool IsTopLevelModuleMap;
+ uint32_t ContentHash[2];
};
bruno wrote:
> aprantl wrote:
> > Why is this not a uint64_t?
> Serializing a `uint64_t` directly instead of two `u
bruno updated this revision to Diff 219168.
bruno added a comment.
Remove pasto from one of the testcases
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org/D67249
Files:
clang/include/clang/Basic/DiagnosticSeriali
bruno updated this revision to Diff 219166.
bruno added a comment.
Update testcase to use a more portable version of `touch`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67249/new/
https://reviews.llvm.org/D67249
Files:
clang/include/clang/Basi
bruno marked 2 inline comments as done.
bruno added a comment.
> Nice! Are you planning to address the FIXME's in a later update of this patch?
The FIXME's are just replaying what the code around does, both error dropping
and `FileEntryRef` are recent changes that didn't make all the way throu
aprantl added a comment.
Nice! Are you planning to address the FIXME's in a later update of this patch?
Comment at: clang/lib/Serialization/ASTWriter.cpp:1767
bool IsTopLevelModuleMap;
+ uint32_t ContentHash[2];
};
Why is this not a uint64_t?
===
bruno created this revision.
bruno added reviewers: dexonsmith, arphaman, rsmith, aprantl.
Herald added subscribers: cfe-commits, jkorous.
Herald added a project: clang.
When files often get touched during builds, the mtime based validation
leads to different problems in implicit modules builds, e
14 matches
Mail list logo