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

This revision was automatically updated to reflect the committed changes. Closed by commit rL316807: [MS] Allow access to ambiguous, inaccessible direct bases (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D39389?vs=120694&id=120713#toc Repository: rL LLVM https://revi

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

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D39365#909912, @compnerd wrote: > @rnk given that the remote unwinder is completely unimplemented ATM, I think > that this isn't a big concern. I'm not sure that this ma

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

rnk added a comment. I'd also like to highlight that LLVM has many users with different needs. Sometimes we need to "disagree and commit". Flags are one of the primary ways that we have to do that. https://reviews.llvm.org/D39079 ___ cfe-commits m

[PATCH] D39210: Add default calling convention support for regcall.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good! Sorry for the delay. https://reviews.llvm.org/D39210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

rnk added a comment. In https://reviews.llvm.org/D38680#903203, @joerg wrote: > I've looked at this in some detail now. I'm not exactly sure yet why it is > broken. The patch seems quite wrong to me. DW_CFA_GNU_args_size should be > applied only when unwinding a call instruction and that regard

[PATCH] D39759: Remove x86 specific code from noplt.c

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Please search for `declare {{.*}}i32 @foo`, and then commit. Otherwise this could match `call i32 @foo` or something like that. https://reviews.llvm.org/D39759 __

[PATCH] D39804: [clang] [python] [tests] Update TLSKind tests for newer MSVC versions

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D39804 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D38221: Add CoreOption flag to "-coverage" option to make it available for clang-cl

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D38221 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39331: Add flags to control the -finstrument-functions instrumentation calls to __cyg_profile functions

rnk added inline comments. Comment at: include/clang/Driver/Options.td:1032 +def fno_instrument_functions_inline : Flag<["-"], "fno-instrument-functions-inline">, Group, Flags<[CC1Option]>, + HelpText<"When using -finstrument-functions, insert the calls after inlining">; ---

[PATCH] D39331: Switch -mcount and -finstrument-functions to emit EnterExitInstrumenter attributes

rnk added inline comments. Comment at: include/clang/Driver/Options.td:1032 +def fno_instrument_functions_inline : Flag<["-"], "fno-instrument-functions-inline">, Group, Flags<[CC1Option]>, + HelpText<"When using -finstrument-functions, insert the calls after inlining">; ---

[PATCH] D39331: Switch -mcount and -finstrument-functions to emit EnterExitInstrumenter attributes

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Thanks, looks good! https://reviews.llvm.org/D39331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good, sorry for the delay! Comment at: include/clang/Basic/LangOptions.def:129 LANGOPT(SjLjExceptions, 1, 0, "setjmp-longjump exception handling") +LANGOPT(SEHExceptions

[PATCH] D39994: Loosen MSVC 2017 path requirements

rnk added a comment. I haven't dug into this code to really understand if this is right and won't change our version detection logic, but yes, broadly I believe we should just consult PATH, find a cl.exe, and check its version. I'd like to reduce this path validation to a minimum. https://rev

[PATCH] D40109: [MS] Apply adjustments after storing 'this'

rnk created this revision. Herald added a subscriber: aprantl. The MS ABI convention is that the 'this' pointer on entry is the address of the vfptr that was used to make the virtual method call. In other words, the pointer on entry always points to the base subobject that introduced the virtual m

[PATCH] D40109: [MS] Apply adjustments after storing 'this'

rnk added a comment. This seems to cause a crash on startup in some gtest binaries when I self-host, so I guess I should debug that tomorrow before committing. The rest of clang's tests pass. I guess we don't use virtual inheritance. =S https://reviews.llvm.org/D40109 __

[PATCH] D39918: [libunwind] Remove a FIXME about truncated section names

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Yep, no need for this. https://reviews.llvm.org/D39918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D40109: [MS] Apply adjustments after storing 'this'

rnk added a comment. In https://reviews.llvm.org/D40109#926975, @rnk wrote: > This seems to cause a crash on startup in some gtest binaries when I > self-host, so I guess I should debug that tomorrow before committing. The > rest of clang's tests pass. I guess we don't use virtual inheritance.

[PATCH] D40109: [MS] Apply adjustments after storing 'this'

This revision was automatically updated to reflect the committed changes. Closed by commit rL318440: [MS] Apply adjustments after storing 'this' (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D40109?vs=123104&id=123218#toc Repository: rL LLVM https://reviews.llvm.org/D

[PATCH] D39588: Distro: initial support for alpine

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D39588 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D40185: Loosen -Wempty-body warning.

rnk requested changes to this revision. rnk added a comment. This revision now requires changes to proceed. Why does passing the rparen location for the if condition fix the warning for IF_ELSE? I assumed that would refer to the else clause semicolon. https://reviews.llvm.org/D40185

[PATCH] D40185: Loosen -Wempty-body warning.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. I'll go ahead and land this as requested to unblock things. Thanks for the patch, sorry for the trouble! Comment at: lib/Sema/SemaChecking.cpp:11818 bool StmtLineInvalid; - un

[PATCH] D40185: Loosen -Wempty-body warning.

rnk updated this revision to Diff 123406. rnk added a comment. - minor tweak https://reviews.llvm.org/D40185 Files: clang/include/clang/Sema/Sema.h clang/lib/Parse/ParseStmt.cpp clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaStmt.cpp clang/test/SemaCXX/warn-empty-body.cpp Index:

[PATCH] D40185: Loosen -Wempty-body warning.

This revision was automatically updated to reflect the committed changes. Closed by commit rL318556: Loosen -Wempty-body warning (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D40185?vs=123406&id=123410#toc Repository: rL LLVM https://reviews.llvm.org/D40185 Files: c

[PATCH] D38445: [x86][inline-asm] allow recognition of MPX regs inside ms inline-asm blob

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good, thanks for the heads up Repository: rL LLVM https://reviews.llvm.org/D38445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D40277: [MS] Increase default new alignment for win64 and test it

rnk created this revision. This raises __STDCPP_DEFAULT_NEW_ALIGNMENT__ from 16 to 32 on Win64. This matches platforms that follow the usual `2 * sizeof(void*)` alignment requirement for malloc. We might want to consider making that the default rather than relying on long double alignment. Fixes

[PATCH] D40277: [MS] Increase default new alignment for win64 and test it

This revision was automatically updated to reflect the committed changes. Closed by commit rL318723: [MS] Increase default new alignment for win64 and test it (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D40277?vs=123692&id=123698#toc Repository: rL LLVM https://revi

[PATCH] D40276: Add -finstrument-function-entry-bare flag

rnk accepted this revision. rnk added a comment. Yep, looks good. :) https://reviews.llvm.org/D40276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

rnk added a comment. In https://reviews.llvm.org/D39673#931487, @martell wrote: > Just as a note there is still a lot to be desired here. I do not particularly > like the `UseSEHExceptions` function default and the actual macro definition > guards should be based on the current `ExceptionModel`

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Yep, looks good Repository: rL LLVM https://reviews.llvm.org/D39673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D137872: Implement lambdas with inalloca parameters by forwarding to function without inalloca calling convention.

rnk added a comment. I'm really happy to see this limitation finally addressed! It's quite a bit of embarrassing tech debt that affects folks making Windows x86 builds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137872/new/ https://reviews.llvm

[PATCH] D153179: [clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

rnk added a comment. Interesting. In Clang, we basically layer the C++ rules over the C rules: if C++ aspects of a class allow it to be passed directly transparently, then we defer down to the C rules, which deal with HVAs, structs, and things like that. In D153179#4429813

[PATCH] D153179: [clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153179/new/ https://reviews.llvm.org/D153179 ___ cfe-c

[PATCH] D153179: [clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

rnk added a comment. The other thing I'm getting at here is that our TargetInfo.cpp abstraction is pretty well built out, even if it's a huge mess. The C++ rules are generally platform neutral, and we don't usually have to resort to these kinds of triple checks, but sometimes it's the most stra

[PATCH] D152986: [clang] Allow 'nomerge' attribute for function pointers

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In D152986#4426256 , @eddyz87 wrote: > In D152986#4425736 , @rnk wrote: > >> The purpose of the attribute is really

[PATCH] D153690: [clang][Sema] Remove dead diagnostic for loss of __unaligned qualifier

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153690/new/ https://reviews.llvm.org/D153690 ___ cfe-c

[PATCH] D153898: [DebugInfo] Enable debug info emission for extern variables in C++

rnk accepted this revision. rnk added subscribers: MaskRay, dblaikie. rnk added a comment. This revision is now accepted and ready to land. This seems reasonable to me, but I want to get a second opinion from @dblaikie and @maskray for some debug info and BPF perspective. Repository: rG LLVM

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added a comment. Can you please add a test case for the issue that caused the revert? I wanted to dig into that to try to understand why the extra copy was being emitted. I think you mentioned it has something to do with increasing the alignment. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D154007: Reland "Try to implement lambdas with inalloca parameters by forwarding without use of inallocas."

rnk added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:5103 -if (Addr.getAlignment() < Align && +if (CallInfo.isDelegateCall()) { + NeedCopy = false; akhuang wrote: > I think the problem is that it tries to do a copy here b

[PATCH] D151393: [CodeGen] Make __clang_call_terminate have an unwind table entry

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Makes sense to me too, but wait for approval by @efriedma Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151393/new/ https://reviews.llvm.org/D151393

[PATCH] D151515: [CodeGen] add additional cast when checking call arguments

rnk added subscribers: efriedma, rjmccall, rnk. rnk added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4376 +.getTypePtr() +->getPointeeOrArrayElementType(); + const Type *CanonicalArgTy = g

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

rnk added a comment. This is pretty cool, even if it's just for data gathering. It's interesting that 29% of .debug_info and .debug_str is for describing class methods, that's pretty significant, even if it's only a 13% reduction across all debug info sections. Repository: rG LLVM Github Mo

[PATCH] D139749: Headers: use C++ inline semantics in C++ mode

rnk added a subscriber: hans. rnk added a comment. This feels to me like we are still working around some incompatibilities between the MSVC intrin.h / intrin0.h model. I would prefer it if we could always use `static inline` consistently in our intrinsic headers so we don't have to worry about

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

rnk edited reviewers, added: hans, efriedma; removed: majnemer, rnk, aeubanks. rnk added a comment. Thanks for working on this, this is also an issue for our users. I've been out on leave. I replaced myself as a reviewer with Hans. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137107/ne

[PATCH] D149997: [clang] [test] Narrow down MSVC specific behaviours from "any windows" to only MSVC/clang-cl

rnk added a comment. In D149997#4337548 , @mstorsjo wrote: > So, the reason why this failed, is that when invoked as `%clang_cc1` in a > MSVC/clang-cl style build, `_MSC_VER` isn't predefined, while `_WIN32` is. > When invoked via the Clang driver inste

[PATCH] D150645: [Driver] Support multi /guard: options

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/D150645/new/ https://reviews.llvm.org/D150645

[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

rnk added a comment. The approach makes sense to me, but can you include test cases to illustrate the IR and code that gets generated? Comment at: clang/lib/CodeGen/CGException.cpp:1833 +/// Find all local variable captures in the statement. +struct ReturnStmtFinder : ConstStm

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

rnk added a comment. Can you expand on the motivation for removing this extension? This doesn't seem to save a lot of code complexity, and it increases migration costs for our users. I'd like to have some motivation for putting them through the trouble of migrating. CHANGES SINCE LAST ACTION

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

rnk added a comment. In D150875#4353398 , @aaron.ballman wrote: > Note, there was an RFC: > https://discourse.llvm.org/t/rfc-can-we-stop-the-extension-to-allow-dereferencing-void-in-c/65708 Thanks, that addresses my concern. Please link to it from the

[PATCH] D150817: Use windows baskslash on anonymous tag locations if using MSVCFormatting and it's not absolute path.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150817/new/ https://reviews.llvm.org/D150817 ___ cfe-c

[PATCH] D119711: Add asan support for MSVC debug runtimes

rnk added subscribers: thieta, mstorsjo. rnk added a comment. + @mstorsjo @thieta Since we were talking about the complexity involved in choosing the correct ASan runtime, there is actually even more involved. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

rnk added a comment. I tend to think that enabled-by-default options which have names that you are supposed to prefix with no are awkward[1], but the compiler ecosystem (GCC & Clang) has lots of those, and perhaps we should lean into it. I like `-g[no-]undefined-methods` the best of Paul's sugg

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

rnk added a comment. In D152017#4401075 , @dblaikie wrote: > Yeah, mixed feelings about a default-on flag, so you have to use `-gno-*`. > Though one name in that form, might be `-gno-canonical-types`. > (pedantically C++ calls these things "member functi

[PATCH] D152472: [Clang][MS] Remove assertion on BaseOffset can't be smaller than Size.

rnk added a comment. Please do extend the simple record layout dumping format. I think it should be relatively simple to add string dumping and parsing to clang/lib/Frontend/LayoutOverrideSource.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

rnk created this revision. rnk added reviewers: nikic, hans. Herald added subscribers: StephenFan, pengfei. Herald added a project: All. rnk requested review of this revision. Herald added a project: clang. It's not exactly clear what the meaning of TypeInfo::AlignRequirement is, so go directly to

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

rnk added a comment. I think there's a fair bit more cleanup and simplification to be done, see asanUsesGlobalsGC and the comments there. We could check CGOpts.DataSections right there, for example, and rip

[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG651e5ae62d29: [MS] Fix passing aligned records by value in some cases (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D152604: [Driver] Default -fsanitize-address-globals-dead-stripping to true for ELF

rnk added a comment. In D152604#4415403 , @MaskRay wrote: > I have thought about not applying `-fsanitize-address-globals-dead-stripping` > in `-fno-data-sections` mode, so that we won't have sections like `.bss.a`, > but the driver complexity/cognitive

[PATCH] D152472: [Clang][MS] Remove assertion on BaseOffset can't be smaller than Size.

rnk added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:3727 +SmallVector Bases; +for (auto Base: Info.CXXInfo->BaseOffsets) { + Bases.push_back(Base.second.getQuantity()); Rather than iterating the map and sorting afterwards, IM

[PATCH] D152472: [Clang][MS] Remove assertion on BaseOffset can't be smaller than Size.

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/D152472/new/ https://reviews.llvm.org/D152472

[PATCH] D152986: [clang] Allow 'nomerge' attribute for function pointers

rnk added a comment. The purpose of the attribute is really limited to preserving source location information on instructions, and this isn't really a supported usage. The BPF backend and verifier needs to learn to tolerate valid LLVM transforms if it wants to be a real LLVM backend. Of course,

[PATCH] D27769: Update MSVC compat docs about debug info.

rnk added inline comments. Comment at: docs/MSVCCompatibility.rst:80 + Windows debuggers and tools like ETW. + There is no way to request DWARF debug info in clang-cl mode, so linking + with either binutils' ld or LLVM's lld won't produce debug info. Work to

[PATCH] D27769: Update MSVC compat docs about debug info.

rnk added inline comments. Comment at: docs/MSVCCompatibility.rst:77 + debug information if ``/Z7`` or ``/Zi`` is passed. Microsoft's link.exe will + transform the CodeView debug information into a PDB that works in modern + Windows debuggers and tools like ETW. Work to teach

[PATCH] D27769: Update MSVC compat docs about debug info.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Thanks! Comment at: docs/MSVCCompatibility.rst:77 + debug information if ``/Z7`` or ``/Zi`` is passed. Microsoft's link.exe will + transform the CodeView debug information into a

[PATCH] D27641: DebugInfo: Added support for Checksum debug info feature (Clang part)

rnk added inline comments. Comment at: test/CodeGen/debug-info-file-checksum.c:1 +// RUN: %clang -emit-llvm -S -g -gcodeview -x c %s.source -o - | FileCheck %s + Instead of %s.source, you can do %S/Inputs/debug-info-file-checksum.c. Comment at:

[PATCH] D27858: Default to standalone debug info on Windows

rnk created this revision. rnk added reviewers: thakis, hans, dblaikie. rnk added a subscriber: cfe-commits. PDB-based debuggers do not support looking up type names across type stream boundaries. There are two ways where users end up being unable to look into variables with types that were requir

[PATCH] D27858: Default to standalone debug info on Windows

rnk added a comment. In https://reviews.llvm.org/D27858#625367, @thakis wrote: > Did you measure what this does to link times? No, but I plan to recover it in Chromium by doing something like: if (is_win && is_clang && !is_win_fastlink) { cflags += ["-flimit-debug-info"] } https://re

[PATCH] D27858: Default to standalone debug info on Windows

rnk added a comment. In https://reviews.llvm.org/D27858#625367, @thakis wrote: > Did you measure what this does to link times? I've measured now, and it's pretty bad. Times to link blink_core increase by 39% with -fstandalone-debug (211s -> 294s). I still think it's what we should do by defau

[PATCH] D27794: Make some diagnostic tests C++11 clean

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Everything here looks good to me. Comment at: test/FixIt/fixit.cpp:28 +// In C++11 this gets 'expected unqualified-id' which fixit can't fix. +#if __cplusplus < 201103L -

[PATCH] D27641: DebugInfo: Added support for Checksum debug info feature (Clang part)

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. This side looks good. Comment at: lib/CodeGen/CGDebugInfo.cpp:325 +llvm::DIFile::ChecksumKind +CGDebugInfo::getChecksum(FileID FID, SmallString<32> &Checksum) const { + Checksum.c

[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

rnk added a comment. How about standardizing on -disable-llvm-passes instead of -disable-llvm-optzns? I never liked "optzns" and can't remember how to spell it. Also, this flag disables LLVM instrumentation passes (ASan) as well as optimization passes, so I think -disable-llvm-passes is the bet

[PATCH] D28053: Cleanup the handling of noinline function attributes, -fno-inline, -fno-inline-functions, -O0, and optnone.

rnk added a comment. The big change here is that `clang -O0` now applies the noinline attribute everywhere. I can see why someone might expect things to work that way, but it seems surprising to me at first glance. Before this change I can imagine someone distributing a static archive of bitco

[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

rnk added a comment. In https://reviews.llvm.org/D28047#629889, @chandlerc wrote: > I only have a mild preference for how we spell the name based on the fact > that i've told people to use -disable-llvm-optzns for years. If we want to go > with ...-passes, fine with me. We have some freedom t

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

rnk added a comment. I think you uploaded the wrong patch on the last upload, the code looks like it's related to __attribute__((deprecated)) now? https://reviews.llvm.org/D27529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D28166: Properly merge K&R functions that have attributes

rnk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:3570 for (auto *A : llvm::make_range(Arg, ArgRange.end())) - ArgTypes.push_back(getVarArgType(A)); + ArgTypes.push_back(getVarArgType(A, CallArgTypeInfo == nullptr)); I'm testin

[PATCH] D28166: Properly merge K&R functions that have attributes

rnk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:3570 for (auto *A : llvm::make_range(Arg, ArgRange.end())) - ArgTypes.push_back(getVarArgType(A)); + ArgTypes.push_back(getVarArgType(A, CallArgTypeInfo == nullptr)); rnk wrote:

[PATCH] D28166: Properly merge K&R functions that have attributes

rnk added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:3570 for (auto *A : llvm::make_range(Arg, ArgRange.end())) - ArgTypes.push_back(getVarArgType(A)); + ArgTypes.push_back(getVarArgType(A, CallArgTypeInfo == nullptr)); rnk wrote:

[PATCH] D28274: [MS] Instantiate default args during instantiation of exported default ctors

rnk created this revision. rnk added a reviewer: rsmith. rnk added a subscriber: cfe-commits. Replace some old code that probably pre-dated the change to delay emission of dllexported code until after the closing brace of the outermost record type. Only uninstantiated default argument expressions

[PATCH] D28271: [Sema] Get rid of unused default argument to Sema::CheckCallingConvAttr.

rnk added a comment. I think you've discovered this bug: struct F { void __attribute__((pcs("aapcs"))) f(); }; void __attribute__((pcs("aapcs"))) F::f() {} Clang ignores the pcs convention on 32-bit x86, and because we don't have an FD here, we choose the wrong default calling convention (n

[PATCH] D28274: [MS] Instantiate default args during instantiation of exported default ctors

rnk updated this revision to Diff 83059. rnk added a comment. simplify https://reviews.llvm.org/D28274 Files: lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaTemplateInstantiateDecl.cpp test/CodeGenCXX/dllexport.cpp test/SemaCXX/default-arg-closures.cpp test/SemaCXX/dllexport.cpp Index: test/S

[PATCH] D28166: Properly merge K&R functions that have attributes

rnk added inline comments. Comment at: lib/Sema/SemaDecl.cpp:11958-11962 +// The type location may be attributed; strip the attributes to get to +// the function type location. +while (auto ATL = TL.getAs()) { + TL = ATL.getModifiedLoc(); +

[PATCH] D28220: provide Win32 native threading

rnk added inline comments. Comment at: include/__threading_support:44 +#define WIN32_LEAN_AND_MEAN +#define VC_EXTRA_LEAN +#include kimgr wrote: > EricWF wrote: > > compnerd wrote: > > > EricWF wrote: > > > > Do these definitions have any affect when `` has alre

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

rnk added inline comments. Comment at: include/typeinfo:75 +#elif defined(_WIN32) +#define _LIBCPP_HAS_WINDOWS_TYPEINFO +#else Is _WIN32 the right condition? It seems like this is intended to match the MS ABI RTTI structure, not the Itanium one. _MSC_VER maybe?

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

rnk added inline comments. Comment at: include/clang/CodeGen/CGFunctionInfo.h:136 } + static ABIArgInfo getDirectHva(llvm::Type *T = nullptr) { +auto AI = getDirect(T); Marking HVAs with inreg is an x86-specific convention. Let's move this down to Target

[PATCH] D28271: [Sema] Get rid of unused default argument to Sema::CheckCallingConvAttr.

rnk added a comment. In general we can't get an FD here because we might be in the middle of parsing the declarator. The calls from SemaType.cpp in particular can't actually provide one. They would have to thread through the IsVariadic and IsCXXMethod bools based on syntactic cues (the ellipsis

[PATCH] D22275: Support -fno-delayed-template-parsing in clang-cl.exe

rnk commandeered this revision. rnk edited reviewers, added: DaveBartolomeo; removed: rnk. rnk added a comment. Looks good, let me grab this and add a test. I think this just got lost in the shuffle over the summer. https://reviews.llvm.org/D22275

[PATCH] D22275: Support -fno-delayed-template-parsing in clang-cl.exe

This revision was automatically updated to reflect the committed changes. Closed by commit rL290990: Support -fno-delayed-template-parsing in clang-cl.exe (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D22275?vs=63713&id=83091#toc Repository: rL LLVM https://reviews.ll

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

rnk added a comment. Last thing, I promise. Comment at: lib/CodeGen/TargetInfo.cpp:1678 bool UsedInAlloca = false; - for (auto &I : FI.arguments()) { -I.info = classifyArgumentType(I.type, State); -UsedInAlloca |= (I.info.getKind() == ABIArgInfo::InAlloca); + if (S

[PATCH] D28318: [TableGen] Only normalize the spelling of GNU-style attributes.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D28318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28317: [Windows] Remove functions in intrin.h that are defined in Builtin.def.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D28317 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27529: Correct Vectorcall Register passing and HVA Behavior

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! https://reviews.llvm.org/D27529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D28274: [MS] Instantiate default args during instantiation of exported default ctors

This revision was automatically updated to reflect the committed changes. Closed by commit rL291045: [MS] Instantiate default args during instantiation of exported default ctors (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D28274?vs=83059&id=83170#toc Repository: rL L

[PATCH] D28383: build: add a heuristic to determine the C++ ABI

rnk added a comment. In https://reviews.llvm.org/D28383#637570, @majnemer wrote: > Why isn't this equivalent to `_MSC_VER` ? It might also be nice for clang to define some macro to indicate which C++ ABI is in use. All that said, I think it makes sense to encode this in libc++ include/__conf

[PATCH] D28320: [Driver] Driver changes to support CUDA compilation on Windows.

rnk added a comment. I went ahead and landed https://reviews.llvm.org/rL291255 to try to fix forward. Take a look and let me know if that's not the right fix. Repository: rL LLVM https://reviews.llvm.org/D28320 ___ cfe-commits mailing list cfe-c

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

rnk added a comment. The new MSVC layout suggests to me that we should look try looking at the path of cl.exe before we open the exe and try to check its version. We'd change this code to inspect the path before looking in the exe: VersionTuple MSVCToolChain::computeMSVCVersion(const Driver *

[PATCH] D26900: [cmake] Obtain LLVM_CMAKE_PATH from llvm-config if available

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm I figure we've waited a few days at this point. https://reviews.llvm.org/D26900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

rnk added inline comments. Comment at: CMakeLists.txt:43 +if (WIN32 AND NOT MINGW) + set(LIBCXX_TARGETING_WINDOWS ON) +else() smeenai wrote: > EricWF wrote: > > EricWF wrote: > > > smeenai wrote: > > > > Not the biggest fan of this name, since it's not obvious w

[PATCH] D28350: [Sema] Avoid -Wshadow warning when a "redefinition of " error is presented

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rL LLVM https://reviews.llvm.org/D28350 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

rnk added a comment. In https://reviews.llvm.org/D20136#640689, @amccarth wrote: > > and folks have to manually add mincore.lib to their link. > > I could load the library dynamically on demand and use GetProcAddress, but I > suspect that would further degrade the performance. I could probably

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

rnk added a comment. In https://reviews.llvm.org/D28365#639253, @hamzasood wrote: > - Added an option to set the environment in a clang::driver::Command, which > makes the environment modifying introduced in the last update a bit more > reliable. > > @rnk I looked into using the new MSVC tool

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

rnk added a comment. In https://reviews.llvm.org/D28365#640825, @hamzasood wrote: > In https://reviews.llvm.org/D28365#640775, @rnk wrote: > > > Can we revert the linker environment change from this patch? It'll be > > easier to review separately. > > > Sure. But that'll break compiling for x86

<    12   13   14   15   16   17   18   19   20   >