[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

arames added a comment. I was was asked to chime in to assess whether this patch could be a problem for the prebuilt-implicit clang modules workflow. No problem here. With prebuilt modules, the output file name has to be specified manually. So the mapping does not change the existing requirement

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

This revision was automatically updated to reflect the committed changes. Closed by commit rG655bea4226b4: [modules] Use `HashBuilder` and `MD5` for the module hash. (authored by arames). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames updated this revision to Diff 369794. arames marked an inline comment as done. arames added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 Files:

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames marked 2 inline comments as done. arames added inline comments. Comment at: clang/include/clang/Basic/ObjCRuntime.h:486 + template + friend void addHash(llvm::HashBuilderImpl , + const ObjCRuntime ) { dexonsmith wrote: > arames

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames added inline comments. Comment at: clang/include/clang/Basic/ObjCRuntime.h:486 + template + friend void addHash(llvm::HashBuilderImpl , + const ObjCRuntime ) { I have added these in the same line as existing `hash_value` functions.

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames updated this revision to Diff 369489. arames added a comment. Fix to native endianness. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 Files: clang/include/clang/Basic/ObjCRuntime.h

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames added inline comments. Comment at: clang/include/clang/Serialization/ModuleFileExtension.h:89 + using ExtensionHashBuilder = + llvm::HashBuilderImpl; + virtual void hashExtension(ExtensionHashBuilder ) const; This obviously needs to be fixed.

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames marked 3 inline comments as done. arames added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4580 - return llvm::APInt(64, code).toString(36, /*Signed=*/false); + return llvm::APInt(64, Hash.getValue()).toString(36, /*Signed=*/false); }

[PATCH] D102943: [modules] Use `HashBuilder` and `MD5` for the module hash.

arames added a comment. I still need to go through earlier comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 ___ cfe-commits mailing list

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames updated this revision to Diff 369203. arames marked an inline comment as not done. arames added a comment. Now using the `HashBuilder` interface. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames updated this revision to Diff 349141. arames added a comment. Fix detail namespace. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 Files: clang/include/clang/Basic/ObjCRuntime.h

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames added inline comments. Comment at: clang/include/clang/Basic/Sanitizers.h:81 + template void updateHash(HashT ) const { +Hash.updateRange([0], [0] + kNumElem); An alternative to having those in class would be to have helpers directly in

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames updated this revision to Diff 349139. arames added a comment. Use `llvm::MD5`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 Files: clang/include/clang/Basic/ObjCRuntime.h

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames added a comment. This new version is an attempt to have modules not rely on `llvm::hash_code`, but on a new `llvm::stable_hash_code`. I understand modifying `ADT/Hashing.h` is sensitive, so maybe we need to discuss the high-level approach first. Repository: rG LLVM Github Monorepo

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames updated this revision to Diff 348406. arames added a comment. Herald added a subscriber: mgorny. Diff against the parent commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 Files:

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames added a comment. The early commits are missing from the PR. Looking out to do this with `arc`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 ___

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames updated this revision to Diff 348403. arames added a comment. Introduce and use `stable_hash_code` instead of modifying `hash_code`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102943/new/ https://reviews.llvm.org/D102943 Files:

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames added a comment. In D102943#2775099 , @dexonsmith wrote: > In D102943#2774732 , @jroelofs > wrote: > >> why do module hashes need to be stable when cross-compiling? > > IIUC, the use case is

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames added a comment. In D102943#2775131 , @dexonsmith wrote: > In D102943#2775115 , @pcc wrote: > >> Isn't the bug here that module hashing is using `hash_code`? So shouldn't >> the correct fix be to use a

[PATCH] D102943: Hashing: use a 64-bit storage type on all platforms.

arames created this revision. Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman, hiraditya. arames requested review of this revision. Herald added projects: clang, LLVM, clang-tools-extra. Herald added subscribers: cfe-commits, llvm-commits. `size_t` varying across platforms can

[PATCH] D90963: Allow searching for prebuilt implicit modules.

arames created this revision. arames added a reviewer: stella.stamenova. Herald added subscribers: cfe-commits, dang. Herald added a project: clang. arames requested review of this revision. This reverts commit c67656b994c87224e0b33e2c4b09093986a5cfa6

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames added a comment. In D68997#2377641 , @stella.stamenova wrote: > This change broke the windows lldb bot: > > http://lab.llvm.org:8011/#/builders/83/builds/570 > > Can you please fix this or revert? Looking at this now. Repository: rG LLVM

[PATCH] D68997: Allow searching for prebuilt implicit modules.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG71e108cd86e7: Allow searching for prebuilt implicit modules. (authored by arames). Changed prior to commit:

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames marked an inline comment as done. arames added a comment. Fixed the trailing whitespace. I also just got commit rights, so I will commit it myself. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 300780. arames added a comment. Rebased on ToT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 Files: clang/docs/Modules.rst clang/include/clang/Driver/Options.td

[PATCH] D88265: [Sema] Support Comma operator for fp16 vectors.

arames added a comment. > Oh I think you'd need to edit the revision on Phabricator (top right on this > page, `edit revision`). But if you are fine with it I can commit the patch > for you with the adjusted commit title. Done ! Thanks for your help ! Repository: rG LLVM Github Monorepo

[PATCH] D88265: Fix comma with half vectors.

arames added a comment. In D88265#2302653 , @fhahn wrote: > Could you adjust the commit message to be a bit more descriptive, e.g > something like `[Sema] Support Comma operator for fp16 vectors.` Done! Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88265: Fix comma with half vectors.

arames updated this revision to Diff 295316. arames added a comment. Update commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88265/new/ https://reviews.llvm.org/D88265 Files: clang/lib/Sema/SemaExpr.cpp

[PATCH] D88265: Fix comma with half vectors.

arames added a comment. I do not have commit rights, so it would be great if you can land it. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88265/new/ https://reviews.llvm.org/D88265 ___

[PATCH] D88265: Fix comma with half vectors.

arames updated this revision to Diff 295116. arames added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88265/new/ https://reviews.llvm.org/D88265 Files: clang/lib/Sema/SemaExpr.cpp

[PATCH] D88265: Fix comma with half vectors.

arames updated this revision to Diff 295025. arames marked an inline comment as done. arames added a comment. Addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88265/new/ https://reviews.llvm.org/D88265 Files:

[PATCH] D88265: Fix comma with half vectors.

arames marked 2 inline comments as done. arames added inline comments. Comment at: clang/test/Sema/fp16vec-sema.c:29 sv0 = hv0 >= hv1; + hv0, 1; // expected-warning 2 {{expression result unused}} sv0 = hv0 || hv1; // expected-error{{logical expression with vector types

[PATCH] D88265: Fix comma with half vectors.

arames created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. arames requested review of this revision. The current half vector was enforcing an assert expecting "(LHS is half vector) == (RHS is half vector)" for comma. Repository: rG LLVM Github

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames added inline comments. Comment at: clang/include/clang/Lex/HeaderSearch.h:187 + /// The hash used for module cache paths. + std::string ModuleHash; + In the previous version of this patch, this value was derived from the stem of `ModuleCachePath`.

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 282336. arames added a comment. Fix a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 Files: clang/docs/Modules.rst clang/include/clang/Driver/Options.td

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 282335. arames marked an inline comment as done. arames added a comment. Addressed review comments. - Fixed typos in the doc. - Added doc about module compatibility. - Cleaned and tested commands in the doc. - Reworked module hash code to not require

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames marked 2 inline comments as done. arames added inline comments. Comment at: clang/docs/Modules.rst:295 + +A trick to prebuilt required modules in one command is to generate implicit modules using the ``-fdisable-module-hash`` option. + Bigcheese wrote: >

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 281602. arames added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 Files: clang/docs/Modules.rst clang/include/clang/Driver/Options.td

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 279263. arames added a comment. Herald added a subscriber: dang. Rebase on top-of-tree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 Files: clang/docs/Modules.rst

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 264096. arames added a comment. Rebase on top of tree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 Files: clang/docs/Modules.rst

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames updated this revision to Diff 264048. arames edited the summary of this revision. arames added a comment. Rebase on top of tree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72100/new/ https://reviews.llvm.org/D72100 Files:

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames updated this revision to Diff 250247. arames added a comment. Apply `clang-format`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72100/new/ https://reviews.llvm.org/D72100 Files: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames updated this revision to Diff 247087. arames added a comment. Rename and clarify. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72100/new/ https://reviews.llvm.org/D72100 Files: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames marked 2 inline comments as done. arames added a comment. In D72100#1855483 , @jkorous wrote: > We should either simplify the implementation to reflect that we don't support > e. g. `*:42` (seems preferable to me) or have the codepaths that are >

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames updated this revision to Diff 239972. arames added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72100/new/ https://reviews.llvm.org/D72100 Files:

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames marked 3 inline comments as done. arames added inline comments. Comment at: clang/test/Frontend/verify-any-file.c:4 +#include "verify-any-file.h" +// expected-error@*:1 {{unknown type name 'unexpected'}} +#include "verify-any-file.h" While testing, I

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames added a comment. This is for example useful to add a catch-all clause like `// expected-note-re@*:* 1+ {{candidate function {{.+` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72100/new/ https://reviews.llvm.org/D72100

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

arames created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. arames added a reviewer: rsmith. This allows specifying `*` for the filename to match any file. For example: // expected-note@*:* {{Match this note in any file at any line.}} //

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames added a comment. Ping. Still looking for someone to take a look. Happy to answer any questions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 ___

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames updated this revision to Diff 228267. arames added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68997/new/ https://reviews.llvm.org/D68997 Files: clang/docs/Modules.rst clang/include/clang/Driver/Options.td

[PATCH] D68997: Allow searching for prebuilt implicit modules.

arames created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. arames edited the summary of this revision. arames added reviewers: bruno, rsmith. Herald added a subscriber: dexonsmith. The behavior is controlled by the `-fprebuilt-implicit-modules` option,