[PATCH] D67249: [Modules][PCH] Hash input files content

2019-10-14 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-10-01 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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 ___

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-10-01 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-13 Thread Juergen Ributzka via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-11 Thread Juergen Ributzka via Phabricator via cfe-commits
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://

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-10 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-10 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-06 Thread Juergen Ributzka via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-06 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-06 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-06 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-05 Thread Adrian Prantl via Phabricator via cfe-commits
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? ===

[PATCH] D67249: [Modules][PCH] Hash input files content

2019-09-05 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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