[PATCH] D75406: Avoid including FileManager.h from SourceManager.h

2020-03-11 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe08464fb4504: Avoid including FileManager.h from SourceManager.h (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D75406?vs=247479&id=249752#toc Repository: rG LLVM Github Monorep

[PATCH] D76040: [TableGen] Move generated *Attr class methods out of line

2020-03-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: aaron.ballman, rjmccall, jdoerfert. Herald added a project: clang. After this change, clang spends ~200ms parsing Attrs.inc instead of ~560ms. A large part of the cost was from the StringSwitch instantiations, but this is a good way to avoid similar

[PATCH] D76040: [TableGen] Move generated *Attr class methods out of line

2020-03-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done. rnk added a comment. The main things going out of line are: - Create* factory methods - Constructors - Enum/string converters (use StringSwitch -> slow to instantiate) Here's two examples from before & after: https://reviews.llvm.org/P8203 https://reviews.ll

[PATCH] D76040: [TableGen] Move generated *Attr class methods out of line

2020-03-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D76040#1919170 , @aaron.ballman wrote: > LGTM as well, though please run clang-format over the patch (some of the > formatting looks to have gone weird in places). Thanks, done. I undid some of its more questionable reformatting

[PATCH] D76040: [TableGen] Move generated *Attr class methods out of line

2020-03-12 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7420f96924a3: [TableGen] Move generated *Attr class methods out of line (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D76040?vs=249817&id=249956#toc Repository: rG LLVM Github

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

2020-03-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 250068. rnk added a comment. - rebase, ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65543/new/ https://reviews.llvm.org/D65543 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Driver/ToolChain.h

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

2020-03-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I believe folks are in agreement about this direction, but I'm waiting for someone to mark the revision accepted. Let me know if that's not the case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65543/new/ https://reviews.llv

[PATCH] D76099: [Clang][Driver] In -fintegrated-cc1 mode, avoid crashing on exit after a compiler crash

2020-03-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm The timer linked list is an interesting complication. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76099/new/ https://reviews.llvm.org/D76099

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

2020-03-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:6682 + /// The outermost level of selector sets. + llvm::SmallVector Sets; + This is not a good data structure choice. You have three levels of small vector nesting, so sizeof OMPTra

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

2020-03-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! I also noticed that Parser.h includes OpenMPClause.h just for this class. OpenMPClause.h is pretty big. It's for this family of related methods: /// Parse a property kind into \p TI

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

2020-03-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76184/new/ https://reviews.llvm.org/D76184

[PATCH] D76289: [NFC][Attr TableGen] Emit Spelling Enum for Attr types if there >1

2020-03-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:2333 -if (!ElideSpelling) - SpellingEnum = CreateSemanticSpellings(Spellings, SemanticToSyntacticMap); -if (Header) I had to call this function during both header and c

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

2020-03-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I patched this in with it's dependency, and to compile just Attr.h by itself, it reduces the time from ~2.420s to ~1.920s, so it is worth pushing this through. :) However, I noticed that there are still some targets broken by the removal of the SmallSet.h include. I'm happ

[PATCH] D76444: Use FinishThunk to finish musttail thunks

2020-03-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: hans, rjmccall. Herald added a project: clang. FinishThunk, and the invariant of setting and then unsetting CurCodeDecl, was added in 7f416cc42638 (2015). The invariant didn't exist when I added this musttail codepath in ab2090d10765 (2014). Recently

[PATCH] D73307: Unique Names for Functions with Internal Linkage

2020-03-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I guess my initial reaction is that it is disappointing that downstream consumers can't cope with non-unique symbol names (What is the point of having internal linkage if we can't rely on it?), but given @mtrofin's input about AFDO, this seems like it may be a practical sol

[PATCH] D60748: Adds an option "malign-pass-aggregate" to make the alignment of the struct and union parameters compatible with the default gcc

2020-03-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:353 +VALUE_LANGOPT(AlignPassingAggregate, 1, 0, "Compatible with gcc default passing struct and union (x86 only).") + If only codegen needs to know, a CodeGenOption would be bette

[PATCH] D71092: [VFS] More consistent support for Windows

2020-01-31 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I was worried there would be more duplication. :) As far as rebuilding the wheel square goes, this seems totally reasonable. I had a question and a nit, though. Comment at: llvm/lib/Support/VirtualFileSystem.cpp:-1113 + sys::path::Style style = sys::

[PATCH] D71092: [VFS] More consistent support for Windows

2020-02-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71092/new/ https://reviews.llvm.org/D71092 ___ cfe-commits mailing list cfe-commit

[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

2020-02-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done. rnk added a comment. Regarding the `Error Err; ... return std::move(Err);` pattern, I think it needs to be written like this for now. At the very least, NRVO is not possible because of the Expected conversion, so std::move is not a pessimization. Thanks for

[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

2020-02-04 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1091730f5fbb: Avoid many std::tie/tuple instantiations in ASTImporter (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D73675?vs=241552&id=242446#toc Repository: rG LLVM Github Mo

[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

2020-02-05 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D73675#1858688 , @shafik wrote: > I forgot to mention this earlier but LLDB is a major user of the > `ASTImporter` and so in general it is a good idea to run `check-lldb` as well. I ran the LLDB tests on Linux, but I get 42 failu

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

2020-02-05 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I applied this locally, but the test doesn't pass. I can look into it tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69763/new/ https://reviews.llvm.org/D69763 ___ cfe-com

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

2020-02-05 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Oh, here's the linker it has found: $ find ../clang/ -iname '*lld-link2' ../clang/test/Driver/Inputs/Windows/ARM/8.1/usr/bin/ld.lld-link2 So this is truly a test of the `ld.otherlinker` feature pattern, not some special case driver feature. I guess we should leave the t

[PATCH] D73865: [CodeGenModule] Assume dso_local for -fpic -fno-semantic-interposition

2020-02-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. What is the motivation for adding a best effort `-fsemantic-interposition`? Was there anything wrong with our previously unstated project policy of ignoring this complexity and surviving without this mode? Less modes -> less conditional soup... Comment a

[PATCH] D74447: [Clang] After integrated-cc1, ignore -disable-free when there are more than one job in the queue

2020-02-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/include/clang/Driver/Job.h:59 /// Whether to print the input filenames when executing. bool PrintInputFilenames = false; Pack a new field after an existing bool for maximum micro optimization. :)

[PATCH] D74452: [MS] Mark vectorcall FP and vector args inreg

2020-02-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: craig.topper, erichkeane. Herald added a project: clang. This has no effect on how LLVM passes the arguments, but it prevents rewriteWithInAlloca from thinking that these parameters should be part of the inalloca pack. Follow-up to D72114

[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

2020-02-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: craig.topper, erichkeane. Herald added a project: clang. This is a follow-up to case 1 in D72114 . The C++ argument classification logic supersedes the C rules, so we have to add a second check for overaligned record

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

2020-02-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Herald added a reviewer: rriddle. Herald added a subscriber: Joonsoo. This revision now requires review to proceed. lgtm Comment at: llvm/lib/Support/Unix/Threading.inc:273 + +int

[PATCH] D74490: [Clang] Limit -fintegrated-cc1 to only one TU

2020-02-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/test/Driver/cc1-spawnprocess.c:9 -// RUN: %clang_cl -fintegrated-cc1 -### -- %s 2>&1 \ +// RUN: %clang_cl -fintegrated-cc1 -c -### -- %s 2>&1 \ // RUN:

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. lgtm The fix looks like what Richard asked for, so I feel comfortable approving this. Thanks for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 __

[PATCH] D74498: [clang-tidy] Fix performance-noexcept-move-constructor-fix.cpp on non-English locale

2020-02-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, if the diff comes out wrong, that will not result in an incorrect pass/fail, but only less debugable output. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74498/new/ https://reviews.

[PATCH] D74634: Remove "ELF Only" restriction from section flags

2020-02-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: hans, aaron.ballman. Herald added subscribers: sunfish, aheejin. Herald added a project: clang. -ffunction-sections and -fdata-sections are well supported by many object file formats: - ELF - COFF - XCOFF - wasm Only MachO ignores this flag. While

[PATCH] D69471: [Coverage] Revise format to reduce binary size

2020-02-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I haven't forgotten about this, even though it's been two months. Hopefully I get to it soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69471/new/ https://reviews.llvm.org/D69471

[PATCH] D74634: Remove "ELF Only" restriction from section flags

2020-02-18 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa0a1f412fd1d: Remove "ELF Only" from -f*-sections help text (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74634/new/ https://reviews.llvm

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

2020-01-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I agree, raising -fgnuc-version is not an acceptable workaround, and we can't ship a clang that doesn't work with old versions of glibc. I think we need a different workaround. The simplest one I can think of is to make `__warn_memset_zero_len` a recognized builtin that gen

[PATCH] D72786: [clang] Set function attributes on SEH filter functions correctly.

2020-01-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, just some test comment tweaks. Comment at: clang/test/CodeGen/exceptions-seh-finally.c:3 +// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -O1 -disable

[PATCH] D72855: Make LLVM_APPEND_VC_REV=OFF affect clang, lld, and lldb as well.

2020-01-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: llvm/cmake/modules/LLVMConfig.cmake.in:81 +set(LLVM_APPEND_VC_REV "@LLVM_APPEND_VC_REV@") + This here should make the standalone build configs w

[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

2020-01-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. `ArmMveAliasValid` is the second slowest function to compile in all of clang, according to ClangBuildAnalyzer: https://reviews.llvm.org/P8185 We shouldn't spend 28 CPU seconds per rebuild on a switch. Please do something to speed it up. Generally, it is best to emit tables

[PATCH] D67161: [clang,ARM] Initial ACLE intrinsics for MVE.

2020-01-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I posted a patch to speed things up: https://reviews.llvm.org/D72984 The numbers are different, since 9 seconds is measured in a local linux build with no debug info, vs. 28s I measured when building at work, on Windows, with debug info. Repository: rG LLVM Github Monore

[PATCH] D72984: [TableGen] Use a table to lookup MVE intrinsic names

2020-01-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: simon_tatham, dmgreen, ostannard, MarkMurrayARM. Herald added a project: clang. Speeds up compilation of SemaDeclAttr.cpp by nine seconds: 0m49.555s - > 0m40.249s Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D72984 Files: c

[PATCH] D72984: [TableGen] Use a table to lookup MVE intrinsic names

2020-01-21 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf63d76373879: [TableGen] Use a table to lookup MVE intrinsic names (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72984/new/ https://revie

[PATCH] D72703: Add a warning, flags and pragmas to limit the number of pre-processor tokens in a translation unit

2020-01-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I waited to see if there was any other feedback, but I'm in favor of this. Should we try to come up with better pragma names? `clang max_tokens` doesn't seem to call to mind what it does: warn if there have been more than this many tokens so far in the translation unit. `ma

[PATCH] D72703: Add a warning, flags and pragmas to limit the number of pre-processor tokens in a translation unit

2020-01-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I like the `max_tokens_here` / `max_tokens_total` variants. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72703/new/ https://reviews.llvm.org/D72703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D72703: Add a warning, flags and pragmas to limit the number of pre-processor tokens in a translation unit

2020-01-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72703/new/ https://reviews.llvm.org/D72703 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

2020-01-23 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2af74e27ed7d: [MS] Overhaul how clang passes overaligned args on x86_32 (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72114/new/ https://

[PATCH] D71313: [AST] Split parent map traversal logic into ParentMapContext.h

2020-01-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 240050. rnk added a comment. This revision is now accepted and ready to land. - Move things around while retaining API compatibility for getParents Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71313/new/ https://r

[PATCH] D71313: [AST] Split parent map traversal logic into ParentMapContext.h

2020-01-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I think I can land this version, but I'll wait until tomorrow so I'll be around to debug fallout. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71313/new/ https://reviews.llvm.org/D71313

[PATCH] D71313: [AST] Split parent map traversal logic into ParentMapContext.h

2020-01-24 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8a81daaa8b58: [AST] Split parent map traversal logic into ParentMapContext.h (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D71313?vs=240050&id=240283#toc Repository: rG LLVM Gi

[PATCH] D73385: [Sema] Split availability processing into SemaAvailability.cpp

2020-01-24 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: arphaman, aaron.ballman, rsmith. Herald added subscribers: dexonsmith, mgorny. Herald added a project: clang. Reduces compile time of SemaDeclAttr.cpp down to 28s from 50s. The new TU does a few RecursiveASTVisitor instantiations, so it takes 30s.

[PATCH] D73385: [Sema] Split availability processing into SemaAvailability.cpp

2020-01-24 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdd8e0a0a23ba: [Sema] Split availability processing into SemaAvailability.cpp (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73385/new/ htt

[PATCH] D72331: OpaquePtr: add type to inalloca attribute.

2020-01-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. lgtm, thanks! I remember reading through this last week, but apparently I did not hit send. This patch actually prompted me to start working on the replacement for this thing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72331/new/ ht

[PATCH] D73202: Make AST reading work better with LLVM_APPEND_VC_REV=NO

2020-01-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73202/new/ https://reviews.llvm.org/D73202 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

2020-01-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. If we can reland this patch, it should fix this new clang bug: https://bugs.llvm.org/show_bug.cgi?id=44705 This was the list of failures you noted on the commit: In rG647c3f4e47de8a850ffcaa897db68702d8d2459a#885042

[PATCH] D73667: Speed up compilation of ASTImporter

2020-01-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: rsmith, aaron.ballman. Herald added a reviewer: martong. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added subscribers: llvm-commits, teemperor. Herald added projects: clang, LLVM. rnk updated this revision to Diff 2412

[PATCH] D73667: Speed up compilation of ASTImporter

2020-01-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 241297. rnk added a comment. Herald added a subscriber: rnkovacs. - remove unintended hack to test MSVC Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73667/new/ https://reviews.llvm.org/D73667 Files: clang/lib/A

[PATCH] D73667: Speed up compilation of ASTImporter

2020-01-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. The `Expected>` and `std::tie` instantiations are still expensive, and they seem avoidable. We could use this code pattern instead: ExpectedType ASTNodeImporter::VisitVariableArrayType(const VariableArrayType *T) { QualType ToElementType = T->getElementType(); E

[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

2020-01-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: rsmith, aaron.ballman, a_sidorin. Herald added a reviewer: martong. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a subscriber: teemperor. Herald added a project: clang. Use in-out parameters to avoid making std::t

[PATCH] D73667: Speed up compilation of ASTImporter

2020-01-30 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGaf3e88495627: Speed up compilation of ASTImporter (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73667/new/ https://reviews.llvm.org/D7366

[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

2020-01-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk planned changes to this revision. rnk marked an inline comment as done. rnk added a comment. In D73675#1849182 , @martong wrote: > Thanks for this patch too! > However, I have a minor concern with this new approach: > If `importSeq` fails then a `ToS

[PATCH] D73675: Avoid many std::tie/tuple instantiations in ASTImporter

2020-01-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 241552. rnk added a comment. - Rewrite with importChecked, no importSeq Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73675/new/ https://reviews.llvm.org/D73675 Files: clang/lib/AST/ASTImporter.cpp Index: clang

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

2020-01-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/lib/CodeGen/CGValue.h:18 #include "clang/AST/ASTContext.h" +#include "clang/Sema/Sema.h" #include "clang/AST/Type.h" This includes Sema.h into every codegen file that uses CGValue.h (most of them). That seems bad fo

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

2020-01-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I moved this include in rG01943a59f51d8b5ede062305941c1f864b8a6a13 . I meant to paste this in the message, but I'll put it here, since the results were significant: $ diff -u deps-before.txt deps-after.

[PATCH] D70950: Add ProcName to TimeTraceProfiler

2020-01-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This broke ClangBuildAnalyzer on Windows because it has a very naive check for "clang": https://github.com/aras-p/ClangBuildAnalyzer/blob/master/src/main.cpp#L148 I was wondering why this wasn't working on Windows anymore. =/ Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D44689: Set dso_local on string literals

2018-03-20 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D44689 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44688: Set dso_local for runtime functions

2018-03-20 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm We don't assume these are `dso_local` on Linux, right? https://reviews.llvm.org/D44688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D44712: Set dso_local on runtime variables

2018-03-20 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D44712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44710: Set dso_local on builtin functions

2018-03-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D44710 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44796: Set dso_local on vtables

2018-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D44796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44805: Set dso_local on __ImageBase

2018-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D44805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk requested changes to this revision. rnk added a comment. This revision now requires changes to proceed. Use `llvm::sys::Process::GetArgumentVector`, which already does wildcard expansion from what I can see. It works with Unicode command lines and isn't affected by locale. Repository: rC

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: tools/clang-format/ClangFormat.cpp:348 + if (EC) { +llvm::errs() << "error: couldn'g get arguments: " << EC.message() << '\n'; + } s/couldn'

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

2018-03-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: rsmith, thakis, hans. This fixes PR33561 and PR34185. Don't store pending template instantiations for late-parsed templates in the normal PendingInstantiations queue. Instead, use a separate list that will only be parsed and instantiated at end of T

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

2018-03-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 139663. rnk added a comment. - make late-parsed templates pending at end of TU prefix https://reviews.llvm.org/D44846 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/test/PCH/late-parse

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

2018-03-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/lib/Sema/Sema.cpp:855 // instantiations. PCH files do not. if (TUKind != TU_Prefix) { DiagnoseUseOfUnimplementedSelectors(); rsmith wrote: > In the TUPrefix case, we'll need to write these instantiations to

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

2018-03-26 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL328567: [MS] Fix late-parsed template infinite loop in eager instantiation (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4

[PATCH] D44846: [MS] Fix late-parsed template infinite loop in eager instantiation

2018-03-26 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC328567: [MS] Fix late-parsed template infinite loop in eager instantiation (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D44846?vs=139663&id=139827#toc Repository:

[PATCH] D44362: [clang] Change std::sort to llvm::sort in response to r327219

2018-03-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D44362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

2018-03-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk resigned from this revision. rnk added a comment. I don't see a new test case for the issue reduced out of Chromium. I'd recommend adding that. I have to resign because I don't know enough about deduction to stamp this... https://reviews.llvm.org/D36918 _

[PATCH] D44723: Set dso_local when clearing dllimport

2018-03-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I'd add more tests but I can't think of ways to remove the dll attributes, only to flip them. Do we already have dso_local tests for this similar case: __declspec(dllimport) void f(); voi

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

2018-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. rnk added reviewers: rjmccall, rsmith, hans. Herald added a subscriber: Prazek. The following class hierarchy requires that we be able to emit a this-adjusting thunk for B::foo in C's vftable: struct Incomplete; struct A { virtual A* foo(Incomplete p) = 0; };

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

2018-03-31 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D45112#1053539, @majnemer wrote: > Does this help PR25641? I think so, these are probably duplicate PRs. I think your analysis on that bug is slightly off, because the TU that provides `ImplCanvas::createColor` *can't* provide the thunks that `

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

2018-04-02 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC329009: [MS] Emit vftable thunks for functions with incomplete prototypes (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D45112?vs=140499&id=140680#toc Repository:

[PATCH] D45112: [MS] Emit vftable thunks for functions with incomplete prototypes

2018-04-02 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL329009: [MS] Emit vftable thunks for functions with incomplete prototypes (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45

[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

2018-04-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: majnemer. rnk added a comment. In https://reviews.llvm.org/D45174#1054820, @rsmith wrote: > +rnk This might also affect the MS ABI, but it does not result in any test > case failures at least (and MSVC's type trait matches our state after this > patch rather than before)

[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Seems reasonable, looks good. Repository: rC Clang https://reviews.llvm.org/D45152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D45152#1056122, @mstorsjo wrote: > In https://reviews.llvm.org/D45152#1055871, @rnk wrote: > > > Seems reasonable, looks good. > > > Any opinion on the wording of the option name? Maybe what we're trying to do is find the sysroot relative to clan

[PATCH] D45234: CMake: Check LLVM_ENABLE_LIBXML2 in clang

2018-04-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. The only think that links against libxml2 is c-index-test. Surely nobody cares about installing c-index-test on some other machine that lacks the local version of libxml2. What's the use case for this to justify the complexity of interacting with LLVM's configuration option

[PATCH] D45233: [Driver] Update GCC libraries detection logic for Gentoo.

2018-04-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. Thousands of lines of C++ to answer one of life's most simple questions: where are the libraries and where are the headers. =p lgtm Comment at: lib/Driver/ToolChains/Gnu.cpp:2275 // CURRENT=triple-version if (Line

[PATCH] D45422: [Driver] Allow drivers to add multiple libc++ include paths

2018-04-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D45422 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D44975: Change DEBUG() macro to LLVM_DEBUG()

2018-04-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D44975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D44976: Change DEBUG() macro to LLVM_DEBUG() in clang-tools-extra

2018-04-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44976 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D45234: CMake: Check LLVM_ENABLE_LIBXML2 in clang

2018-04-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D45234 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[PATCH] D45500: [MinGW] Look for libc++ headers in a triplet prefixed path as well

2018-04-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/Driver/ToolChains/MinGW.cpp:459 addSystemInclude(DriverArgs, CC1Args, + Base + Arch + llvm::sys::path::get_separator() + +

[PATCH] D44646: Sema: in msvc compatibility mode, don't allow forceinline on variadics

2018-04-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:3889 + if (Context.getTargetInfo().getCXXABI().isMicrosoft() && + hasFunctionProto(D) && isFunctionOrMethodVariadic(D)) { Why is this warning dependent on the ABI? GCC has a similar warning:

[PATCH] D45523: [CodeGen] Handle __func__ inside __finally

2018-04-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Comment at: test/CodeGen/exceptions-seh-finally.c:281 +// CHECK-LABEL: define internal void @"?fin$0@0@finally_with_func@@"({{[^)]*}}) +// CHECK: call void @cleanup_w

[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.

2017-10-25 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D39224#906225, @ilya-biryukov wrote: > In https://reviews.llvm.org/D39224#905431, @rnk wrote: > > > Can you remind me why `NamedDecl::printQualifiedName` is not appropriate > > for your needs? > > > The use-case in code completion (see https://rev

[PATCH] D39224: Moved QualTypeNames.h from Tooling to AST.

2017-10-25 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good https://reviews.llvm.org/D39224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D38819: [libunwind] Add support for dwarf unwinding on windows on x86_64

2017-10-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D38819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

2017-10-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Doesn't this change the ABI considerably? I suspect Apple cares about that. Is remotely unwinding a 64-bit thread from a 32-bit process a concern? That's the main use case that forcing 64-bit words seems to enable. https://reviews.llvm.org/D39365 ___

[PATCH] D39389: [MS] Allow access to ambiguous, inaccessible direct bases

2017-10-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. Clang typically warns that in the following class hierarchy, 'A' is inaccessible because there is no series of casts that the user can write to access it unambiguously: struct A { }; struct B : A { }; struct C : A, B { }; MSVC allows the user to convert from C* t

<    11   12   13   14   15   16   17   18   19   20   >