[PATCH] D45978: dllexport const variables must have external linkage.

rnk added a comment. I still feel like there has to be a more uniform way to handle this. Basically anything with __declspec(dllexport) on it is effectively upgraded to external linkage. Comment at: lib/Sema/SemaDeclAttr.cpp:6491 + if (auto *VD = dyn_cast(D)) { +if (getL

[PATCH] D57004: [docs] Add release notes for notable things I've contributed since last release

rnk added inline comments. Comment at: docs/ReleaseNotes.rst:190 +- For MinGW, clang now produces vtables and RTTI for dllexported classes + without key functions. + hans wrote: > mstorsjo wrote: > > This comment doesn't really say much for the casual reader, pe

[PATCH] D57189: Fix compatibility with the msvc AI compiler option

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, I'll go ahead and commit this soon. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57189/new/ https://reviews.llvm.org/D57189 ___

[PATCH] D57189: Fix compatibility with the msvc AI compiler option

rnk updated this revision to Diff 183398. rnk added a comment. - add test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57189/new/ https://reviews.llvm.org/D57189 Files: clang/include/clang/Driver/CLCompatOptions.td clang/test/Driver/cl-options.c Index: clang/test/Driver/cl-optio

[PATCH] D57189: Fix compatibility with the msvc AI compiler option

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 rL352119: [clang-cl] Ignore space-separated /AI arguments (authored by rnk, committed by ). Herald added a subscriber: llvm-

[PATCH] D57208: Replace two RecursiveASTVisitor insantiations with StmtVisitor

rnk created this revision. rnk added reviewers: arphaman, rsmith. RecursiveASTVisitor is very expensive to instantiate and results in needlessly slow compilation. For these availability check fixits, we don't need to instantiate the full complexity of the declaration walking machinery, we can use

[PATCH] D57209: Revert "[AArch64] Use LL for 64-bit intrinsic arguments"

rnk added a comment. I'd say go for it, it's basically already EOD Pacific time, and I don't think anyone has set up a continuous Windows ARM64 build with ToT clang anywhere in the world yet. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57209/new/ https://revi

[PATCH] D57209: Revert "[AArch64] Use LL for 64-bit intrinsic arguments"

rnk added a comment. In D57209#1370789 , @mstorsjo wrote: > In D57209#1370608 , @rnk wrote: > > > and I don't think anyone has set up a continuous Windows ARM64 build with > > ToT clang anywhere in the world yet. >

[PATCH] D57268: [dllimport] Don't use RAV to check inlinability

rnk created this revision. rnk added a reviewer: hans. I don't like this change, because I had to copy a fair amount of logic from RAV into the dllimport visitor. However, the build time and size results were still impressive, so I wanted to upload this and get a second opinion. Here's the data:

[PATCH] D57114: Remove Expr sugar decorating the CXXUuidofExpr node.

rnk added a comment. Thanks for following up! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57114/new/ https://reviews.llvm.org/D57114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D53541: [COFF, ARM64] Support SEH for ARM64 Windows

rnk added a comment. In https://reviews.llvm.org/D53541#1282764, @mgrang wrote: > Updated the patch with the following changes: > > 1. Emit llvm.x86.seh.recoverfp only for non-aarch64 targets. For aarch64 > windows, the parent fp is always passed in x1. So we don't need a separate > instrinsic

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

rnk added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; takuto.ikuta wrote: > hans wrote: > > takuto.ikuta wrote: > > > h

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

rnk added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:246 + /// If set, dllexported classes dllexport their inline methods. + bool DllExportInlines = true; + We should define this in the LangOptions.def file. https://reviews.llvm.org/

[PATCH] D53985: Use C++11 fallthrough attribute syntax when available and add a break

rnk created this revision. rnk added reviewers: EricWF, ldionne. Herald added subscribers: erik.pilkington, christof. This silences the two -Wimplicit-fallthrough warnings clang finds in ItaniumDemangle.h in libc++abi. Clang does not have a GNU attribute spelling for this attribute, so this is ne

[PATCH] D53985: Use C++11 fallthrough attribute syntax when available and add a break

rnk added subscribers: aaron.ballman, rsmith. rnk added a comment. In https://reviews.llvm.org/D53985#1284196, @ldionne wrote: > Would it make sense to add the GNU spelling to the attribute in Clang? I'll redirect that question to @rsmith and @aaron.ballman. I have a vague recollection that it

[PATCH] D53985: Use C++11 fallthrough attribute syntax when available and add a break

This revision was automatically updated to reflect the committed changes. Closed by commit rL345870: Use C++11 fallthrough attribute syntax when available and add a break (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org

[PATCH] D53950: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rC345882: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC (authored by rnk, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D53950?vs=1

[PATCH] D53950: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rL345882: Fix clang -Wimplicit-fallthrough warnings across llvm, NFC (authored by rnk, committed by ). Herald added a subscriber: jrtc27. Changed prior to commit: https://reviews.llvm.org/D53950?vs=172208

[PATCH] D54046: [COFF, ARM64] Implement InterlockedExchange*_* builtins

rnk accepted this revision. rnk added a comment. lgtm Repository: rC Clang https://reviews.llvm.org/D54046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53912: [Headers] [MS] Add intrin0.h

rnk added a subscriber: STL_MSFT. rnk added a comment. In https://reviews.llvm.org/D53912#1284804, @azharudd wrote: > I agree. This currently resolves issues with building for ARM64 target using > Visual Studio 2017. The missing intrinsics it complains about are already > present in intrin.h. W

[PATCH] D54062: [COFF, ARM64] Implement InterlockedCompareExchange*_* builtins

rnk added inline comments. Comment at: include/clang/Basic/BuiltinsARM.def:270 +TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_nf, "LLiLLiD*LLiLLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_rel, "LLiLLiD*LLiLLi", "nh", "

[PATCH] D54062: [COFF, ARM64] Implement InterlockedCompareExchange*_* builtins

rnk added inline comments. Comment at: include/clang/Basic/BuiltinsARM.def:270 +TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_nf, "LLiLLiD*LLiLLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "") +TARGET_HEADER_BUILTIN(_InterlockedCompareExchange64_rel, "LLiLLiD*LLiLLi", "nh", "

[PATCH] D54171: [MS] Zero out ECX in __cpuid in intrin.h

rnk created this revision. rnk added reviewers: craig.topper, hans. Some CPUID leafs depend on the value of ECX as well as EAX, but we left it uninitialized. Originally reported as https://crbug.com/901547 https://reviews.llvm.org/D54171 Files: clang/lib/Headers/intrin.h clang/test/CodeGen

[PATCH] D54171: [MS] Zero out ECX in __cpuid in intrin.h

This revision was automatically updated to reflect the committed changes. Closed by commit rL346265: [MS] Zero out ECX in __cpuid in intrin.h (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54171?vs=172811&id=172831#t

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

rnk created this revision. rnk added reviewers: aprantl, zturner. Herald added subscribers: JDevlieghere, mgorny. This is an initial prototype of how we can run debugger integration tests on Windows. cdb and windbg share a command language and debugger engine. Visual Studio has its own, but we sho

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

rnk added a comment. In https://reviews.llvm.org/D54187#1289607, @aprantl wrote: > It would be nice if instead of having a set of windows-only tests, we could > wrap cdb similar to llgdb.py wraps LLDB, so these tests run on all platforms. > If the Windows debugger is just too different for this

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

rnk added a comment. I hadn't realized that Dexter knew how to drive VS tools. I'll have to go read more and get back to you all. I think it would be more promising than attempting to come up with a new llgdb.py-like abstraction for cdb. Specifically, abstracting over setting breakpoints and r

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

rnk added a comment. Threading a new options argument through mangleType that includes QualifierMangleMode as well as these obj-c options seems reasonable. Repository: rC Clang https://reviews.llvm.org/D52674 ___ cfe-commits mailing list cfe-com

[PATCH] D54536: [AST] Fix typo in MicrosoftMangle

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I'd say feel free to commit typo fixes like this in the future. I'd avoid fixing typos where there is spelling disagreement, like "prolog vs prologue", so just use your best judgement as always. lgt

[PATCH] D54499: [codeview] Make "clang -g" emit codeview by default when targetting MSVC

This revision was automatically updated to reflect the committed changes. Closed by commit rC346907: [codeview] Make "clang -g" emit codeview by default when targetting MSVC (authored by rnk, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D54425: [AArch64] Add aarch64_vector_pcs function attribute to Clang

rnk added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:1101 case CC_AAPCS: + case CC_AArch64VectorCall: return llvm::dwarf::DW_CC_LLVM_AAPCS; sdesmalen wrote: > I wasn't really sure whether this requires a corresponding DW_CC_LLVM_AAVPCS > r

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

rnk added a comment. Well, you could go further down the route of what we do for "structors", and store the top-level decl being mangled in the mangler. Would that solve the problem? Repository: rC Clang https://reviews.llvm.org/D52674 ___ cfe-

[PATCH] D55229: [COFF, ARM64] Make -flto-visibility-public-std a driver and cc1 flag

rnk added a comment. I think @compnerd arranged things this way. I don't know why -flto-visibility-public-std is involved, I have no idea what that does. I think we should do what MSVC does here, which is to leave _CxxThrowException unannotated and let the linker synthesize import thunks. Here

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added a comment. Sorry, I was out sick for a week. We should ask @smeenai about this change, since he has been doing more work in this area recently. Comment at: CodeGen/CodeGenModule.cpp:2957-2958 !getCodeGenOpts().LTOVisibilityPublicStd && - !getTripl

[PATCH] D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword.

This revision was automatically updated to reflect the committed changes. Closed by commit rC354838: [MS] Fix for Bug 8446, template instantiation without a 'typename' keyword (authored by rnk, committed by ). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rC

[PATCH] D58292: Add support for importing ChooseExpr AST nodes.

rnk added a comment. I had to revert this in rL354839 because one of the tests didn't pass on Windows: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/4641 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58292/new/

[PATCH] D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword.

rnk added a comment. In D41950#1409332 , @zahiraam wrote: > I don't think I have commit right can you please commit it. Thanks. Sure, done! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41950/new/ https://reviews.llvm.o

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

rnk added a comment. Looks good and thorough, but it needs tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58530/new/ https://reviews.llvm.org/D58530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D58691: [MS] Don't emit coverage for deleting dtors

rnk created this revision. rnk added a reviewer: vsk. Herald added a subscriber: jdoerfert. Herald added a project: clang. The MS C++ ABI has no constructor variants, but it has destructor variants, so we should move the deleting destructor variant check outside the check for "does the ABI have co

[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

rnk added a comment. Sounds good, thanks for debugging this. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58663/new/ https://reviews.llvm.org/D58663 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D58691: [MS] Don't emit coverage for deleting dtors

rnk added inline comments. Comment at: clang/test/Profile/cxx-abc-deleting-dtor.cpp:28 +// FIXME: Should we emit coverage info for deleting dtors? They do contain +// conditional branches. LLVM IR PGO will insrument them just fine, though. + vsk wrote: > Probably

[PATCH] D58691: [MS] Don't emit coverage for deleting dtors

rnk updated this revision to Diff 188434. rnk marked 3 inline comments as done. rnk added a comment. - fix CHECK-NOT - FIXME that isn't needed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58691/new/ https://reviews.llvm.org/D58691 Files: clang/

[PATCH] D58691: [MS] Don't emit coverage for deleting dtors

This revision was automatically updated to reflect the committed changes. Closed by commit rL354924: [MS] Don't emit coverage for deleting dtors (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

rnk created this revision. rnk added reviewers: xur, vsk. Herald added subscribers: Sanitizers, cfe-commits, hiraditya, eraman. Herald added projects: clang, Sanitizers, LLVM. I hadn't realized that instrumentation runs before inlining, so we can't use the function as the comdat group. Doing so ca

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

rnk marked 2 inline comments as done. rnk added a comment. In D58737#1412734 , @vsk wrote: > I'm confused by this wording re: comdats in the LangRef: "All global objects > that specify this key will only end up in the final object file if the linker > ch

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

rnk updated this revision to Diff 188635. rnk marked an inline comment as done. rnk added a comment. - share code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58737/new/ https://reviews.llvm.org/D58737 Files: clang/test/Profile/cxx-templates.cp

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

rnk added a comment. Oops, forgot to respond to this... In D58737#1412734 , @vsk wrote: > I'm confused by this wording re: comdats in the LangRef: "All global objects > that specify this key will only end up in the final object file if the linker > choo

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

rnk added a comment. In D58737#1412847 , @xur wrote: > lgtm. > > BTW, I'm in the process of committing D54175 > . After that patch, PGO instrumentation can > be called after the main inlining. I don't think it will confli

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

This revision was automatically updated to reflect the committed changes. Closed by commit rL355044: [InstrProf] Use separate comdat group for data and counters (authored by rnk, committed by ). Herald added a subscriber: delcypher. Changed prior to commit: https://reviews.llvm.org/D58737?vs=18

[PATCH] D58844: Give builtins and alloc/dealloc operators the default calling convention.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58844/new/ https://reviews.llvm.org/D58844 ___ cfe-commits mailing

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Would you like someone to land this for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58530/new/ https://reviews.llvm.org/D58530 __

[PATCH] D59118: creduce script for clang crashes

rnk added a subscriber: gbiv. rnk added a comment. @gbiv already got all my shell quoting comments. I think we should do one more round of fixes, we can commit that for you, and then move on to the next steps. Comment at: clang/utils/creduce-clang-crash.py:109 + open(testfil

[PATCH] D59118: creduce script for clang crashes

rnk added inline comments. Comment at: clang/utils/creduce-clang-crash.py:1 +#!/usr/bin/env python +# A tool that calls C-Reduce to create a minimal reproducer for clang crashes george.burgess.iv wrote: > nit: please specify a python version here, since the world

[PATCH] D58544: [AST] Improve support of external layouts in `MicrosoftRecordLayoutBuilder`

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/AST/RecordLayoutBuilder.cpp:2750-2753 +// It is possible that there were no fields or bases located after vbptr, +// so the size was not adjusted befor

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

rnk added a comment. Very cool! I'll take a look, I wasn't aware this had been rebased and uploaded, I was thinking about doing it myself yesterday as a side project. As I think I've said elsewhere, I'm really excited to give users the tools they need to analyze why their code compiles slowly.

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

rnk added inline comments. Comment at: clang/lib/Parse/ParseAST.cpp:172 + { +llvm::TimeTraceScope scope("Backend", ""); +Consumer->HandleTranslationUnit(S.getASTContext()); I think you may want to move this to `clang::EmitBackendOutput`, which is closer

[PATCH] D58530: Add PragmaHandler for MSVC pragma execution_character_set

This revision was automatically updated to reflect the committed changes. Closed by commit rL356185: Add PragmaHandler for MSVC pragma execution_character_set (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

rnk added inline comments. Comment at: llvm/lib/Support/TimeProfiler.h:1 +//===- llvm/Support/TimeProfiler.h - Hierarchical Time Profiler -*- C++ -*-===// +// I applied this patch locally to try it, and I noticed this header should be in llvm/include/llvm/Suppo

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

rnk added inline comments. Comment at: llvm/lib/IR/LegacyPassManager.cpp:1686 + llvm::TimeTraceScope timeScope("OptModule", M.getName().data()); for (Function &F : M) I think these OptModule and OptFunction labels may need some improvement. For a backend-h

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good to me. Let's wait for @jyknight though. Comment at: lib/Headers/ia32intrin.h:120 + +#ifndef _MSC_VER +/* Select the correct function based on the size of long. */ --

[PATCH] D58160: MS ABI: adding template static member in the linker directive section to make sure init function can be called before main.

rnk added a comment. Apparently I wrote this comment long ago and never hit send. Comment at: lib/CodeGen/CGDeclCXX.cpp:484 AddGlobalCtor(Fn, 65535, COMDATKey); +if (getTarget().getCXXABI().isMicrosoft()) { + // In The MS C++, MS add template static data member in

[PATCH] D58160: MS ABI: adding template static member in the linker directive section to make sure init function can be called before main.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/CodeGen/CGDeclCXX.cpp:484 AddGlobalCtor(Fn, 65535, COMDATKey); +if (getTarget().getCXXABI().isMicrosoft()) { + // In The MS C++, MS add template

[PATCH] D59440: add steps to preprocess file and reduce command line args

rnk added inline comments. Comment at: clang/utils/creduce-clang-crash.py:106-117 + # Check that an empty file is not interesting + # file_to_reduce is hardcoded into the test, so this is a roundabout + # way to run it on an empty file + _, tmpfile = tempfile.mkstemp() + _,

[PATCH] D58827: [Sema][NFCI] Don't allocate storage for the various CorrectionCandidateCallback unless we are going to do some typo correction

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I think this is worth the complexity of the repeated clone methods. lgtm Comment at: lib/Sema/SemaType.cpp:5911 ExprResult AddrSpace = S.ActOnIdExpression( - S.getC

[PATCH] D59440: add steps to preprocess file and reduce command line args

rnk added a comment. Aside from the buglet, I'm happy with this, but I wanted to wait until @george.burgess.iv takes a look. Comment at: clang/utils/creduce-clang-crash.py:44 + +def quote_cmd(cmd): + for i, arg in enumerate(cmd): This modifies cmd in place, s

[PATCH] D59560: Permit redeclarations of a builtin to specify calling convention.

rnk added inline comments. Comment at: lib/Sema/SemaDecl.cpp:3144 + << FunctionType::getNameForCallConv(NewTypeInfo.getCC()) + << 3 /*on builtin function*/; + NewTypeInfo = NewTypeInfo.withCallingConv(OldTypeInfo.getCC()); You can make thes

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

rnk added a comment. I see! I only took a quick look, but this looks exactly like what @rsmith has been asking for in discussions for a long time now: a more explicit AST representation of uuid of uuidof in template arguments. I'll make an effort to get his attention and see if this addresses h

[PATCH] D59560: Permit redeclarations of a builtin to specify calling convention.

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/D59560/new/ https://reviews.llvm.org/D59560 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D59624: [Driver] Pass -malign-double from the driver to the cc1 command line

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/D59624/new/ https://reviews.llvm.org/D59624 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

rnk added a comment. Other than the lifetime issue, I think this is basically ready. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2014 + llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() { + return Instantiation->getQualifiedNameAsString().c_str(); +}

[PATCH] D56803: clang -dumpversion returns 4.2.1 for legacy reason, update it

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Code lgtm, with a suggestion to tighten the test. Comment at: lib/Driver/Driver.cpp:1631 -// introduce a non-pedantically GCC compatible mode to Clang in which we -// provi

[PATCH] D56803: clang -dumpversion returns 4.2.1 for legacy reason, update it

rnk added a comment. In D56803#1441876 , @sylvestre.ledru wrote: > @rnk btw, do you think it should be added to the clang release notes? I think it is release-note worthy. If you change __VERSION__, you can mention that as well. Com

[PATCH] D59725: Additions to creduce script

rnk added a comment. I recently discovered NamedTemporaryFile, maybe that would help simplify up the various mkstemp usages. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59725/new/ https://reviews.llvm.org/D59725 ___ cfe-commits mailing l

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

rnk updated this revision to Diff 192223. rnk added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. - replace struct with current size Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59797/new/ https://reviews.llvm.o

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

rnk marked 4 inline comments as done. rnk added inline comments. Comment at: lld/COFF/Chunks.cpp:47 +namespace { +// This class exists just for the purpose of calculating the expected size of ruiu wrote: > rnk wrote: > > ruiu wrote: > > > rnk wrote: > > > > rui

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

rnk updated this revision to Diff 192752. rnk added a comment. - Replace std::vector<> with singly linked list Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59797/new/ https://reviews.llvm.org/D59797 Files: lld/COFF/Chunks.cpp lld/COFF/Chunks.

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

rnk added a comment. I added one last size optimization, replacing a std::vector with a forward linked list and a custom iterator for it. PTAL Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59797/new/ https://reviews.llvm.org/D59797

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I think this is ready. We can adjust the overloads after the fact. I'd like to get the feature in so we can make improvements independently. Comment at: clang/lib/Parse/Pars

[PATCH] D60018: [codeview] Remove Type member from CVRecord

rnk updated this revision to Diff 193191. rnk marked 2 inline comments as done. rnk added a comment. Herald added subscribers: lldb-commits, cfe-commits, kadircet, arphaman, jkorous. Herald added projects: clang, LLDB. - fix one lldb usage Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D60094: [MSVC] If unable to find link.exe relative to MSVC, look for link.exe in the path

rnk added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:493 C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found); + linkPath = TC.GetProgramPath("link.exe"); +} amccarth wrote: > The comment above explains one reason why we shoul

[PATCH] D59797: [COFF] Reorder fields in Chunk and SectionChunk to reduce their size

rnk added a comment. I think I've addressed all the comments, and I know that @ruiu is currently travelling or recovering from travel to Japan, so I'll go ahead and land this. Feel free to provide post-commit feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D59797: [COFF] Reduce the size of Chunk and SectionChunk, NFC

rnk updated this revision to Diff 193371. rnk added a comment. - last update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59797/new/ https://reviews.llvm.org/D59797 Files: lld/COFF/Chunks.cpp lld/COFF/Chunks.h lld/COFF/ICF.cpp lld/COFF/Ma

[PATCH] D59797: [COFF] Reduce the size of Chunk and SectionChunk, NFC

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 rL357535: [COFF] Reduce the size of Chunk and SectionChunk, NFC (authored by rnk, committed by ). Changed prior to commit:

[PATCH] D60018: [codeview] Remove Type member from CVRecord

rnk updated this revision to Diff 193388. rnk marked 4 inline comments as done. rnk added a comment. - Add RecordPrefix ctor, remove all dummy RecordLen assignments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60018/new/ https://reviews.llvm.org/D

[PATCH] D60018: [codeview] Remove Type member from CVRecord

rnk updated this revision to Diff 193390. rnk marked an inline comment as done. rnk added a comment. - one more change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60018/new/ https://reviews.llvm.org/D60018 Files: lld/COFF/PDB.cpp lldb/source

[PATCH] D60018: [codeview] Remove Type member from CVRecord

rnk added inline comments. Comment at: llvm/include/llvm/DebugInfo/CodeView/CVRecord.h:29 +/// Carrying the size separately instead of trusting the size stored in the +/// record prefix provides some extra safety and flexibility. template class CVRecord { agane

[PATCH] D60018: [codeview] Remove Type member from CVRecord

rnk marked 2 inline comments as done. rnk added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60018/new/ https://reviews.llvm.org/D60018 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D60018: [codeview] Remove Type member from CVRecord

rnk updated this revision to Diff 193526. rnk added a comment. - final updates Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60018/new/ https://reviews.llvm.org/D60018 Files: lld/COFF/PDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordC

[PATCH] D59797: [COFF] Reduce the size of Chunk and SectionChunk, NFC

rnk added a comment. In D59797#1452440 , @MaskRay wrote: > > this reduces the sum of heap allocations ... These numbers exclude memory > > mapped files > > May I ask how you counted the memory usage minus memory mapped files? I'm using the Visual Studio

[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

rnk added inline comments. Comment at: clang/lib/CodeGen/CGAtomic.cpp:1691 } else { -// Build new lvalue for temp address +// Build new lvalue for temp address. Address Ptr = Atomics.materializeRValue(OldRVal); I don't have an issue with these cha

[PATCH] D60018: [codeview] Remove Type member from CVRecord

This revision was automatically updated to reflect the committed changes. Closed by commit rLLD357658: [codeview] Remove Type member from CVRecord (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D60018?vs=193526&id=193633#toc Repository: rLLD LLVM Linker

[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

rnk added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4382-4383 + // Add metadata for calls to MSAllocator functions + if (!DisableDebugInfo) { +if (TargetDecl && TargetDecl->hasAttr()) + getDebugInfo()->addHeapAllocSiteMetadata(CI, RetTy, Loc); -

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

rnk added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:429-430 + const FileEntry *fileEntry = SM.getFileEntryForID(foundIdFromLoc); + if (!fileEntry || fileEntry->getName().empty() || + fileEntry->getName().equals(FileName)) { +CSKind = computeChecksum(foun

[PATCH] D60094: [MSVC] If unable to find link.exe relative to MSVC, look for link.exe in the path

rnk added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:493 C.getDriver().Diag(clang::diag::warn_drv_msvc_not_found); + linkPath = TC.GetProgramPath("link.exe"); +} mstorsjo wrote: > rnk wrote: > > amccarth wrote: > > > The comment a

[PATCH] D60247: Make SourceManager::createFileID(UnownedTag, ...) take a const llvm::MemoryBuffer*

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/D60247/new/ https://reviews.llvm.org/D60247 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added a comment. This patch fixes a lot of LNK4286 warnings when running `check-asan` on Windows, so I'd like to get it committed upstream. Are there any remaining objections? Is it OK if I commandeer the revision and make some minor aesthetic adjustments and land it? CHANGES SINCE LAST A

[PATCH] D59744: Fix i386 ABI "__m64" type bug

rnk added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:9473-9474 IsWin32FloatStructABI, CodeGenOpts.NumRegisterParameters)); +} else if (Triple.getOS() == llvm::Triple::Linux) { + // System V i386 ABI requires __m64 value passing by MMX registers. +

[PATCH] D60237: [MS] Add metadata for __declspec(allocator)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, with some suggestions to improve the test Comment at: clang/test/CodeGen/debug-info-codeview-heapallocsite.c:16-19 +struct Foo foo_buf[1024]; +__declspec(allocator) struct Fo

[PATCH] D60283: [clang-cl] Don't emit checksums when compiling a preprocessed CPP

rnk added a comment. +@rsmith for the PresumedLoc change From glancing on the PresumedLoc computation code, I think this bool might be the way to go. You could make it a bit more "free" by stealing a bit from the column, if we're concerned about size. FYI, I'm off to EuroLLVM after this and re

[PATCH] D60800: [MS] Emit S_HEAPALLOCSITE debug info

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1966 + QualType PointeeTy = D.getTypePtr()->getPointeeType(); + llvm::DIType *DI = getOrCreateType(PointeeTy, getOrCreateFile(Loc)); + CI->setMetadata("heapallocsite", DI); akhuang wrote:

<    1   2   3   4   5   6   7   8   9   10   >