[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

2023-06-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG03a0f4b61ca5: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

2023-06-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/unittests/Tooling/DependencyScannerTest.cpp:274 +llvm::ErrorOr> +openFileForRead(const Twine &Path) override { + ReadFiles.push_back(Path.str()); benlangmuir w

[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

2023-06-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 534123. akyrtzi added a comment. For the test also check for unnecessary `stat` call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153670/new/ https://reviews.llvm.org/D153670 Files: clang/lib/Lex/HeaderSea

[PATCH] D153670: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files

2023-06-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `HeaderSearch::loadSubdirectoryModuleMaps` `stat`s all the files in a directory which causes the dependency scanning servic

[PATCH] D150473: [clang/Driver] Also consider `gnu++` standard when checking for modules support

2023-05-18 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5e975d4f67c0: [clang/Driver] Also consider `gnu++` standard when checking for modules support (authored by akyrtzi). Changed prior to commit: http

[PATCH] D150473: [clang/Driver] Also consider `gnu++` standard when checking for modules support

2023-05-18 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Ping 🙏 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150473/new/ https://reviews.llvm.org/D150473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D150473: [clang/Driver] Also consider `gnu++` standard when checking for modules support

2023-05-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D150473 Files: clang/lib/Driver/ToolChains/Clang.

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

2023-04-30 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. `CodeGen` has the same issue: $ ninja tools/clang/lib/CodeGen/CMakeFiles/obj.clangCodeGen.dir/TargetInfo.cpp.o In file included from /llvm-project/clang/lib/CodeGen/TargetInfo.cpp:36: /llvm-project/llvm/include/llvm/TargetParser/RISCVTargetParser.h:32:10: fatal er

[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

2023-04-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Hi @craig.topper , this patch is causing a build failure: In file included from /llvm-project/clang/lib/Sema/SemaType.cpp:43: /llvm-project/llvm/include/llvm/TargetParser/RISCVTargetParser.h:32:10: fatal error: 'llvm/TargetParser/RISCVTargetParserDef.inc' file not fo

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

2023-04-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 515085. akyrtzi added a comment. Rebase on top of `main`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148369/new/ https://reviews.llvm.org/D148369 Files: clang/lib/Tooling/DependencyScanning/ModuleDepColle

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

2023-04-17 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:905 + if (BriefResult) { +llvm::outs() << "num modules: " << FD->getNumModules() << '\n'; +return HadErrors; jansvoboda11 wrote: > akyrtzi wrote: > > jansvoboda11 wr

[PATCH] D147815: [clang][deps] Print timing information

2023-04-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. > Could you also add a -terse option, to avoid printing the full dependency info Note I added something like this in https://reviews.llvm.org/D148369, so ignore it for this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

2023-04-14 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 513828. akyrtzi added a comment. Remove `-optimize-args` from the test invocations since it's not relevant for the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148369/new/ https://reviews.llvm.org/D1483

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

2023-04-14 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 513751. akyrtzi added a comment. Make sure to check `FD` is valid. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148369/new/ https://reviews.llvm.org/D148369 Files: clang/lib/Tooling/DependencyScanning/Modul

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

2023-04-14 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:905 + if (BriefResult) { +llvm::outs() << "num modules: " << FD->getNumModules() << '\n'; +return HadErrors; jansvoboda11 wrote: > This assumes `FD` is not empty, i.

[PATCH] D148369: [DependencyScanning] Canonicalize `CodeGenOptions.RelaxAll`

2023-04-14 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is particularly useful to avoid diverging the modules between a PCH and a translation-unit compilation. Repository:

[PATCH] D147815: [clang][deps] Print timing information

2023-04-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Could you also add a `-terse` option, to avoid printing the full dependency info, if you mainly want to get the timing? Something like this: T.stopTimer(); if (PrintTiming) llvm::errs() << llvm::format( "clang-scan-deps timing: %0.2fs wall, %0.2fs process

[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-03-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a subscriber: bnbarham. akyrtzi added inline comments. Comment at: clang/lib/Index/USRGeneration.cpp:1032 + case TemplateArgument::UncommonValue: +// FIXME: Visit value. +break; erichkeane wrote: > bolshakov-a wrote: > > aaron.ballman wrote

[PATCH] D145473: [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78a` commit

2023-03-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGb3283bf192c6: [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78… (authored by akyrtzi). Rep

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

2023-03-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. > Is this observable from a clang command line tool? Be nice to have a lit test. Added lit test here: https://reviews.llvm.org/D145473 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145256/new/ https://reviews.llvm.org/D145

[PATCH] D145473: [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78a` commit

2023-03-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145473 Files: clang/test/ARCMT/verify.m Index: clang/t

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

2023-03-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5e035651fd3a: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage`… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

2023-03-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 502201. akyrtzi added a comment. Avoid passing a new `IgnoringDiagConsumer` for the test since it's unused. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145256/new/ https://reviews.llvm.org/D145256 Files: c

[PATCH] D145256: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string

2023-03-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. "But when would you have a completely empty diagnostic message", you ask dear reader? That is when there is an empty "#warn

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-02-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D141910#4112144 , @domada wrote: > @akyrtzi Thank you for your feedback. Can I land the patch? Fine be me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/ https://reviews.llvm.org/D141910

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-02-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D141910#4111048 , @domada wrote: > Added changes in `clang/lib/AST/CMakeLists.txt` to address build issue > reported by @akyrtzi . > > I modified CMakeLists.txt so that it requires generation of missing > `Attributes.inc`. >

[PATCH] D143461: [ClangScanDeps] Fix data race in `clang-scan-deps` tool

2023-02-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi abandoned this revision. akyrtzi added a comment. In D143461#4110405 , @benlangmuir wrote: > Dupe of https://reviews.llvm.org/D143428 ? Indeed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143461

[PATCH] D143461: [ClangScanDeps] Fix data race in `clang-scan-deps` tool

2023-02-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Found using TSan. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D143461 Files: clang/tools/clang-scan

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

2023-02-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. > would be great to test it more in a semantic way if possible Keep in mind that the specific order of the decls doesn't matter for the purposes of this test, what matters is that the order is the same every time for the same input. I personally think that the addition

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-01-31 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. I've reverted this change from `main` branch, let me know if there's anything I can do to help with addressing the build issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/ https://reviews.llvm.org/D141910 __

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

2023-01-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Hi @domada, these changes break compilation of clang, with such build error: FAILED: tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTContext.cpp.o In file included from /llvm-project/clang/lib/AST/ASTContext.cpp:81: In file included from /llvm-project/llvm/i

[PATCH] D142238: [clang/CodeGenActionTest] Use the platform's path separator for the `DebugInfoCWDCodeGen` test

2023-01-20 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb2b078adc2d0: [clang/CodeGenActionTest] Use the platform's path se

[PATCH] D142236: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules`

2023-01-20 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1ff8a687ae15: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules` (authored by akyrtzi). Repository: rG LLVM Github Monorepo

[PATCH] D142238: [clang/CodeGenActionTest] Use the platform's path separator for the `DebugInfoCWDCodeGen` test

2023-01-20 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes a failure in some Windows configuration. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142238 Fi

[PATCH] D142236: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules`

2023-01-20 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142236 Files: clang/include/clang/Driver/Options

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

2023-01-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. akyrtzi marked an inline comment as done. Closed by commit rGed6d09dd4ead: [Lex] For dependency directive lexing, angled includes in `__has_include`… (authored by akyrt

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

2023-01-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4415 +if (Result.isNot(tok::header_name)) + return true; +// Advance the index of lexed tokens. benlangmuir wrote: > This case is missing a t

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

2023-01-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 490629. akyrtzi added a comment. Add test case for dependency directive lexing of ill-formed include inside `__has_include` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142143/new/ https://reviews.llvm.org/D1

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

2023-01-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4420 + DepDirectives.front().Tokens[NextDepDirectiveTokenIndex]; + if (BufferStart + NextTok.Offset >= BufferPtr) +break; benlangmuir wrote: > How do we know this will termin

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

2023-01-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. rdar://104386604 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D142143 Files: clang/lib/Lex/Lexer.cpp

[PATCH] D142028: [clang] remove SUBMODULE_TOPHEADER

2023-01-18 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. This is useful for functionality, like in an IDE, to show the top headers of a module import for navigational purposes. Is it reasonable to at least have an alternative implementation for it (e.g. from the module headers find the headers that were not included from other

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

2023-01-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Yeah, you mainly need more than 32 decls to exceed the small storage of `Scope::DeclSetTy`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D141625 __

[PATCH] D141625: [DeclContext] Sort the Decls before adding into DeclContext

2023-01-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Is it impractical to add a test for this? Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141625/new/ https://reviews.llvm.org/D

[PATCH] D140164: [clang/Lexer] Enhance `Lexer::getImmediateMacroNameForDiagnostics` to return a result from non-file buffers

2022-12-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG59df56413bdc: [clang/Lexer] Enhance `Lexer::getImmediateMacroNameForDiagnostics` to return a… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D140164: [clang/Lexer] Enhance `Lexer::getImmediateMacroNameForDiagnostics` to return a result from non-file buffers

2022-12-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 483370. akyrtzi added a comment. Adjust code comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140164/new/ https://reviews.llvm.org/D140164 Files: clang/lib/Lex/Lexer.cpp clang/test/Modules/build-fail

[PATCH] D140164: [clang/Lexer] Enhance `Lexer::getImmediateMacroNameForDiagnostics` to return a result from non-file buffers

2022-12-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use `SourceManager::isWrittenInScratchSpace()` to specifically check for token paste or stringization, instead of excluding

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

2022-12-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: llvm/include/llvm/Support/PGOOptions.h:18 #include "llvm/Support/Error.h" +#include "llvm/Support/VirtualFileSystem.h" steven_wu wrote: > akyrtzi wrote: > > akyrtzi wrote: > > > I'd suggest to consider moving the `PGO

[PATCH] D139115: [clang][ExtractAPI] Add support for single symbol SGF

2022-12-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/include/clang-c/Documentation.h:549 +typedef struct CXAPISetImpl *CXAPISet; + benlangmuir wrote: > @dang please document what this type represents. > > @akyrtzi what's the preferred implementation type name for

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

2022-12-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1729 + if (!Opts.ProfileInstrumentUsePath.empty()) { +auto FS = llvm::vfs::getRealFileSystem(); +setPGOUseInstrumentor(Opts, Opts.ProfileInstrumentUsePath, *FS, Diags);

[PATCH] D139052: [NFC][Profile] Access profile through VirtualFileSystem

2022-12-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/include/clang/CodeGen/BackendUtil.h:14 #include "llvm/IR/ModuleSummaryIndex.h" +#include "llvm/Support/VirtualFileSystem.h" #include Recommend to forward declare instead of including the header. ==

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-11 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. @nikic, it seems to have recovered (http://llvm-compile-time-tracker.com/compare.php?from=c49cde6467f9bf200640db763152a9dc7f009520&to=0456acbfb942f127359a8defd1b4f1f44420df3e&stat=instructions) let me know if you have concerns. Repository: rG LLVM Github Monorepo CH

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

2022-10-11 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0456acbfb942: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

2022-10-11 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4715 + return createVFSFromOverlayFiles(CI.getHeaderSearchOpts().VFSOverlayFiles, + Diags, BaseFS); +} A bit nitpick but I'd suggest changing t

[PATCH] D135634: [clang][modules] Serialize VFS overlay paths into PCMs

2022-10-11 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. This seems fine to me but note that we no longer depend on the functionality that `test/Index/index-module-with-vfs.m` is testing (and not sure anyone else does), so if there is another change affecting it that is more complicated we could consider removing the test.

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

2022-10-10 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D135490#3847454 , @steven_wu wrote: > I don't know too much about this. I guess have a DiagReceiver to force the > emitting order is a good thing but it is kind of weird to have this just for > unused warning. > > I am guessi

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D135118#3840259 , @akyrtzi wrote: > In D135118#3839442 , @nikic wrote: > >> FYI this caused a noticeable compile-time regression (about 0.4% geomean at >> `-O0`): >> http://llvm-compi

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

2022-10-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a subscriber: mgrang. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Commit `371883f46dc23f8464cbf578e2d12a4f92e61917` caused a noticeable compile-time regre

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D135118#3839442 , @nikic wrote: > FYI this caused a noticeable compile-time regression (about 0.4% geomean at > `-O0`): > http://llvm-compile-time-tracker.com/compare.php?from=92233159035d1b50face95d886901cf99035bd99&to=37188

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-05 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG371883f46dc2: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics (authored by akyrtzi). Repository: rG LLVM Github Monorepo

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:1091 virtual void collectPropertiesToImplement(PropertyMap &PM, PropertyDeclOrder &PO) const {} --

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 465241. akyrtzi added a comment. Herald added subscribers: steakhal, martong. Herald added a reviewer: NoQ. Remove `PropertyDeclOrder` parameter from the `collectPropertiesToImplement` functions. This is not necessary with `PropertyMap` becoming a `MapVector`

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:1091 virtual void collectPropertiesToImplement(PropertyMap &PM, PropertyDeclOrder &PO) const {} benlangmuir wrote: > Can we use the exi

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D135118#3833978 , @steven_wu wrote: > LGTM. > > `RemoveDecl` does become more expensive but I don't have better solution. Luckily AFAICT this is not a "hot" function. > I am also wondering if as follow up we should add an opt

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

2022-10-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a subscriber: mgrang. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. In the context of caching clang invocations it is important to emit diagnostics in deter

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb340c5ae4221: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D133674#3784710 , @akyrtzi wrote: > In D133674#3784602 , @jansvoboda11 > wrote: > >> Could you explain why this is necessary and even correct? I'd expect Clang >> to give an error whe

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 459888. akyrtzi added a comment. Add comment to clarify why we skip if `tok::hashhash` is encountered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133674/new/ https://reviews.llvm.org/D133674 Files: clang/

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D133674#3784602 , @jansvoboda11 wrote: > Could you explain why this is necessary and even correct? I'd expect Clang to > give an error when seeing `##` in this position, and I'd expect the scanner > to do the same. `##` is

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-11 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D133674 Files: clang/lib/Lex/DependencyDirectivesScanner.

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaa484c90cf59: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between… (authored by akyrtzi). Repository: rG LLVM Github Mo

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 458499. akyrtzi added a comment. Remove leftover doc-comment parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 Files: clang/include/clang/Lex/DependencyDir

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked 4 inline comments as done. akyrtzi added a comment. @benlangmuir see latest diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 ___ cfe-commits

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 458360. akyrtzi added a comment. Always print `tokens_present_before_eof` and print it as "" for clarity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 Files: cl

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// benlangmuir wrote: > benlangmuir wrote: > > aky

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// benlangmuir wrote: > Why would you want to prin

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 458173. akyrtzi added a comment. Remove a couple of unused local variables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 Files: clang/include/clang/Lex/Dependenc

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

2022-09-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a subscriber: mgorny. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Directive `dependency_directives_scan::tokens_present_before_eof` is introduced to indic

[PATCH] D133229: [driver] Prune module-map related flags, if they are not going to be needed

2022-09-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D133229#3768101 , @rsmith wrote: > I think the approach you're taking here is probably doomed -- too many things > in Clang depend on whether we've read module map files, and it seems unlikely > to me that you'll be able to c

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-09-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D132801#3761253 , @akyrtzi wrote: > In D132801#3760014 , @rsmith wrote: > >> This doesn't look right to me -- we still use module maps when modules are >> disabled to enforce layering

[PATCH] D133229: [driver] Prune module-map related flags, if they are not going to be needed

2022-09-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. This is follow-up from https://reviews.llvm.org/D132801, but taking into account the conditions where the module-ma

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-08-31 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D132801#3760014 , @rsmith wrote: > This doesn't look right to me -- we still use module maps when modules are > disabled to enforce layering checking, and when > `-fmodules-local-submodule-visibility` is enabled but `-fmodule

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-08-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. akyrtzi marked an inline comment as done. Closed by commit rG33162a81d4c9: [driver] Additional ignoring of module-map related flags, if modules are… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-08-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/test/Driver/modules.m:81 // RUN: %clang -fno-modules -fmodules-validate-system-headers -### %s 2>&1 | FileCheck -check-prefix=VALIDATE_SYSTEM_FLAG %s // VALIDATE_SYSTEM_FLAG-NOT: -fmod

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-08-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 456483. akyrtzi added a comment. Merge the new `RUN` line together with the prior 2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132801/new/ https://reviews.llvm.org/D132801 Files: clang/lib/Driver/ToolCha

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

2022-08-27 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D132801 Files: clang/lib/Driver/ToolChains/Clang.

[PATCH] D131124: [Serialization] Remove `ORIGINAL_PCH_DIR` record

2022-08-05 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6635f48e4aba: [Serialization] Remove `ORIGINAL_PCH_DIR` record (authored by akyrtzi). Changed prior to commit: https://reviews.llvm.org/D131124?vs=449808&id=450439#toc Repository: rG LLVM Github Mono

[PATCH] D131124: [Serialization] Remove `ORIGINAL_PCH_DIR` record

2022-08-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use of `ORIGINAL_PCH_DIR` record has been superseeded by making PCH/PCM files with relocatable paths at write time. Removin

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG944a86de7c50: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D130710#3685436 , @benlangmuir wrote: > Is the functionality provided by `ORIGINAL_PCH_DIR` still useful for making > it possible to move a PCH and its referenced headers? It's not completely > clear to me when this feature

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-29 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 448715. akyrtzi added a comment. Add `FIXME` comment to consider either removing `ORIGINAL_PCH_DIR` or changing the default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130710/new/ https://reviews.llvm.org/D

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-28 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D130710#3685509 , @v.g.vassilev wrote: > In D130710#3685470 , @akyrtzi wrote: > >> @v.g.vassilev is the functionality of "write `ORIGINAL_PCH_DIR` and resolve >> headers relative to i

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-28 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a subscriber: rmaz. akyrtzi added a comment. Also pinging @rmaz who made a related change , is this used by Facebook? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130710/new/ https://reviews.llvm.or

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-28 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a subscriber: v.g.vassilev. akyrtzi added a comment. @v.g.vassilev is the functionality of "write `ORIGINAL_PCH_DIR` and resolve headers relative to it if PCH file and headers moved together" used by `Cling`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

2022-07-28 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is useful to enable sharing of the same PCH file even when it's intended for a different output path. The only inform

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

2022-07-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D130443#3680753 , @thakis wrote: > Looks like this doesn't build: http://45.33.8.238/linux/82380/step_4.txt Sorry about that, fixed here: https://github.com/llvm/llvm-project/commit/c5ddacb3b6afe2fd507b5f4a10c32ec00ffb245e

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

2022-07-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8dfaecc4c244: [CGDebugInfo] Access the current working directory from the `VFS` (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

2022-07-24 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ...instead of calling `llvm::sys::fs::current_path()` directly. Repository: rG LLVM Github Monorepo https://reviews.llv

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

2022-07-18 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D129912#3661181 , @hubert.reinterpretcast wrote: > The added test is not passing on the AIX builder: > https://lab.llvm.org/buildbot/#/builders/214/builds/2388/steps/6/logs/FAIL__Clang-Unit__83 > > Note that Clang on that pla

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

2022-07-18 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfbbabd4ca06a: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to… (authored by akyrtzi). Repository: rG LLVM Github Monorep

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

2022-07-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also include a unit test to validate that the `vfs::FileSystem` object is properly used. Repository: rG LLVM Github Mon

  1   2   3   >