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

2017-01-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D28365#640868, @hamzasood wrote: > Ha, good point. Does that include the environment stuff in Command too or > just the linker? Yes, please make the Command environment changes as part of a separate patch with the linker environment changes.

[PATCH] D30947: [Driver] Fix arch-specific-libdir-rpath.c

2017-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 https://reviews.llvm.org/D30947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 91749. rnk added a comment. - Test that explicit casts suppress the warning https://reviews.llvm.org/D30923 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/SemaCXX/warn-bitfield-en

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D30923#700708, @hfinkel wrote: > In https://reviews.llvm.org/D30923#700696, @rnk wrote: > > > Do you think it's worth indicating that the error can be suppressed with an > > explicit cast, or is that wasted space? > > > What might this look like?

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk marked an inline comment as done. rnk added a comment. Cool, thanks for the reviews, this is definitely in better shape now. This is off by default, so I'm going to commit and experiment with it on a few codebases. I'd still like to hear if either of the Richards have diagnostic wording su

[PATCH] D30923: Warn on enum assignment to bitfields that can't fit all values

2017-03-14 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL297761: Warn on enum assignment to bitfields that can't fit all values (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D30923?vs=91749&id=91752#toc Repository: rL LLVM https://r

[PATCH] D31162: IRGen: Do not set dllexport on declarations.

2017-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/D31162 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31174: [X86][MS-compatability] allow MS TYPE/SIZE/LENGTH operators as a part of a compound expression

2017-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 Repository: rL LLVM https://reviews.llvm.org/D31174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

2017-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I think you need some driver logic to make sure the xray lists show up in .d files so that modifications to them trigger rebuilds in most build systems. See the SanitizerArgs.cpp driver logic for this. Comment at: include/clang/Basic/XRayFunctionFilter.h:

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

2017-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Basic/XRayFunctionFilter.cpp:21 +: AlwaysInstrument( + llvm::SpecialCaseList::createOrDie(AlwaysInstrumentPaths)), + NeverInstrument(llvm::SpecialCaseList::createOrDie(NeverInstrumentPaths)), Hm, phab

[PATCH] D30018: [XRay] Add __xray_customeevent(...) as a clang-supported builtin

2017-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:2748 +if (const auto *XRayAttr = +this->CurFuncDecl->getAttr()) { + if (XRayAttr->neverXRayInstrument()) Don't need `this->` Comment at: lib/CodeGen/CGBuiltin

[PATCH] D31248: [X86] Implement __readgsqword (and the rest) as builtins (PR32373)

2017-03-22 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Yep, thanks, looks good! Repository: rL LLVM https://reviews.llvm.org/D31248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

2017-03-29 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/CodeGen/CGCoroutine.cpp:225 + void Emit(CodeGenFunction &CGF, Flags) override { +CGF.EmitStmt(Deallocate); + } This will be called twice

[PATCH] D31460: [coroutines] Add cleanup for compiler injected objects/allocations in coroutine body

2017-03-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/CGCoroutine.cpp:225 + void Emit(CodeGenFunction &CGF, Flags) override { +CGF.EmitStmt(Deallocate); + } GorNishanov wrote: > rnk wrote: > > This will be called twice: once for a normal exit and once for exce

[PATCH] D30388: [XRay] Add -fxray-{always, never}-instrument= flags to clang

2017-03-29 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: include/clang/Driver/XRayArgs.h:22 +class XRayArgs { + std::vector AlwaysInstrumenFiles; + std::vector NeverInstrumentFiles; Any reason to omit

[PATCH] D31167: Use FPContractModeKind universally

2017-03-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217 /// Adjust BinaryOperator::FPFeatures to match the bit-field size of this. - unsigned fp_contract : 1; + LangOptions::FPContractModeKind fp_contract : 2; }; Please do not

[PATCH] D31491: [XRay][clang] Fix the -fxray-instruction-threshold flag processing

2017-03-30 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: test/CodeGen/xray-instruction-threshold.cpp:11 + +// CHECK-DAG: define i32 @_Z3foov() #[[THRESHOLD:[0-9]+]] { +// CHECK-DAG: define i32 @_Z3barv() #[[NEVERATTR:[0-

[PATCH] D31167: Use FPContractModeKind universally

2017-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217 /// Adjust BinaryOperator::FPFeatures to match the bit-field size of this. - unsigned fp_contract : 1; + LangOptions::FPContractModeKind fp_contract : 2; }; anemet wrote:

[PATCH] D31518: Try to fix the libcxx build with mingw64

2017-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. Herald added a subscriber: mgorny. mingw64 has lots of default libs that libc++ and its test programs should link against. With this patch, cmake now runs successfully with GCC on Windows. https://reviews.llvm.org/D31518 Files: libcxx/cmake/config-ix.cmake Index:

[PATCH] D31518: Try to fix the libcxx build with mingw64

2017-03-30 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL299144: Try to fix the libcxx build with mingw64 (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D31518?vs=93568&id=93570#toc Repository: rL LLVM https://reviews.llvm.org/D31518

[PATCH] D31235: Enhance -Wshadow to warn when shadowing typedefs or type aliases

2017-03-30 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 to me Comment at: lib/Sema/SemaDecl.cpp:6710-6711 + + // Return false if warning is ignored. + if (Diags.isIgnored(diag::warn_decl_shadow, R.getNameLoc())) +re

[PATCH] D29654: [OpenMP] Integrate OpenMP target region cubin into host binary

2017-03-31 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Driver/ToolChains/Cuda.cpp:324 + "CUDA toolchain not expected for an OpenMP host device."); + if (JA.isDeviceOffloading(Action::OFK_OpenMP)) { +if (Output.isFilename()) { This entire if block constructs a co

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

2017-04-05 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:570 +llvm::AtomicOrdering::SequentiallyConsistent); +llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit); +llvm::Value *Truncated = Can you comment that this shifts the relevant bit

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

2017-04-06 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/CodeGen/CGBuiltin.cpp:570 +llvm::AtomicOrdering::SequentiallyConsistent); +llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit); +llvm::Value *

[PATCH] D31702: Append -w when LLVM_ENABLE_WARNINGS is Off.

2017-04-06 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/D31702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31696: Automatically add include-what-you-use for when building in tree

2017-04-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: clang/tools/.gitignore:12 +#==# +# The include-what-you-use project, for when building in-tree. +include-what-you-use LLVM doesn't currently have an

[PATCH] D32014: Remove unused varible

2017-04-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. Looks good https://reviews.llvm.org/D32014 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

2017-04-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D32064#726861, @pcc wrote: > I think it would be better to move this logic to the driver and have it pass > in an `-mllvm` flag. The sanitizer passes should really be taking no > arguments in the constructor like the other passes, so I don't want

[PATCH] D32138: clang-cl: Support the /Zc:twoPhase[-] command-line option (PR32680)

2017-04-17 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. Let's add it now. If we don't add this, some user will run into it first and have to wait for a fix. If we add it and get the meaning wrong, we'll still need to fix it and push a new release, so we'

[PATCH] D32064: [asan] Disable ASan global-GC depending on the target and compiler flags

2017-04-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Driver/SanitizerArgs.cpp:636 + case llvm::Triple::COFF: +return DataSections; + case llvm::Triple::ELF: eugenis wrote: > rnk wrote: > > We can return true for COFF here. By adding a comdat during asan > > instrume

[PATCH] D32014: Remove unused varible

2017-04-18 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL300572: Remove unused varible (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D32014?vs=95123&id=95602#toc Repository: rL LLVM https://reviews.llvm.org/D32014 Files: cfe/trunk

[PATCH] D28590: [Sema] Restrict explicit instantation definition dllexport

2017-01-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, thanks for the fix! https://reviews.llvm.org/D28590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

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

2017-01-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Driver/MSVCToolChain.cpp:34 + #if 0 +#define USE_VS_SETUP_CONFIG + #endif What are the outstanding issues preventing you from enabling this by default? Comment at: lib/Driver/MSVCToolChain.cpp:16

[PATCH] D28705: [Sema] Fix bug in handling of designated initializer

2017-01-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. What happens with virtual bases? struct B { int x; }; struct D : virtual B { int y; }; void test() { D d = {1, .y = 2}; } Comment at: lib/Sema/SemaInit.cpp:2250 +if (auto *CXXRD = dyn_cast(RT->getDecl())) + FieldIndex += CXXRD->getNumBases

[PATCH] D28705: [Sema] Fix bug in handling of designated initializer

2017-01-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. lgtm https://reviews.llvm.org/D28705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29198: clang-cl: Warn about /U flags that look like filenames (PR31662)

2017-01-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/D29198 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29152: Drop 'dllimport' when redeclaring inline function template without the attribute (PR31695)

2017-01-30 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/D29152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-01-30 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/MSVCToolChain.cpp:34 + #if 0 +#define USE_VS_SETUP_CONFIG + #endif hamzasood wrote: > rnk wrote: > > What are the outstanding iss

[PATCH] D29304: [cmake] Hint find_package() to prefer LLVM installed alongside clang

2017-01-30 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: rL LLVM https://reviews.llvm.org/D29304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D29350: clang-cl: Evaluate arguments left-to-right in constructor call with initializer list (PR31831)

2017-01-31 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 Can you check if we got this wrong in clang 3.8 and 3.9? I'm wondering if this was a regression, or if we always got this wrong. https://reviews.llvm.org/D29350 ___

[PATCH] D28989: [X86][MS]Adjacent comments within multi-line inline assembly statement

2017-02-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. Sorry, missed this, looks good. Repository: rL LLVM https://reviews.llvm.org/D28989 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

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

2017-02-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This is ready to land. Do you need someone to commit this? https://reviews.llvm.org/D28365 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-02-02 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL293923: [Driver] Updated for Visual Studio 2017 (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D28365?vs=86502&id=86865#toc Repository: rL LLVM https://reviews.llvm.org/D28365

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

2017-02-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I had to revert this because it doesn't pass tests on Linux. Can you look into that and resubmit after fixing those test failures? Repository: rL LLVM https://reviews.llvm.org/D28365 ___ cfe-commits mailing list cfe-commits@

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

2017-02-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Doesn't your fix mean that the tests will fail on a Windows machine that doesn't have VS because LLVM was built with mingw? Usually in these situations we provide some way to provide a fake toolchain. https://reviews.llvm.org/D28365 _

[PATCH] D29520: Lit C++11 Compatibility - Microsoft diagnostics

2017-02-06 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/D29520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29770: [Assembler] Inline assembly diagnostics test.

2017-02-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: test/Misc/inline-asm-diags.c:1 +// RUN: not %clang -c --target=armv7a-arm-none-eabi %s -o /dev/null 2>&1 | FileCheck %s + Use `clang -cc1 -S -o /dev/null -verify` to test this. https://reviews.llvm.org/D29770 _

[PATCH] D34018: Support __float128 on NetBSD libstdc++ x86/x86_64

2017-06-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I think there's something wrong with clang's implementation of __float128. It seems fundamentally incompatible with libstdc++'s __is_floating_point_helper mechanism. We can definitely take this patch to unblock things, but we should file a bug about investigating this all t

[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax

2017-06-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1382 +if (const MCConstantExpr *CE = +dyn_cast_or_null(Val)) { + StringRef ErrMsg; rnk wrote: > Please use clang-format here and elsewhere not addressed

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

2017-06-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Did you locally add a test case for the dllimport member function pointer template argument? Comment at: lib/Sema/SemaTemplate.cpp:5704 + else +NPV = isNullPointerValueTemplateArgument(S, Param, ParamType, ResultArg); + Think we shoul

[PATCH] D34810: [Sema] -Wcomma should not warn for expressions that return void

2017-06-29 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I thought the intention of -Wcomma was to warn on practically all non-macro uses of the comma operator. I know it's silly to cast void to void, but I seem to recall that this was an intentional style-ish warning like -Wparentheses, which encourages `if ((x = y))` for intent

[PATCH] D34850: [CodeGen] Propagate dllexport to thunks

2017-06-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This appears to affect our behavior under the MS ABI. We still seem to use this logic from CGVTables, even though that code is primarily Itanium related. In http://crbug.com/738468 we're seeing the LNK4102 linker warning, which says: "export of deleting destructor (name); i

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

2017-06-30 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Have a minute to get to this? My attempt to work around the bug in Chromium was incomplete. https://reviews.llvm.org/D34714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D35056: GCC ABI incompatibility when passing object with trivial copy ctor, trivial dtor, and non-trivial move ctor

2017-07-07 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I discussed this with @rsmith and we think the correct fix is to update the DefinitionData bits computed when adding special members. I haven't reviewed this patch in detail, but we came to the conclusion that Clang's optimization to avoid implicit special member declaratio

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

2017-07-09 Thread Reid Kleckner via Phabricator via cfe-commits
rnk commandeered this revision. rnk edited reviewers, added: majnemer; removed: rnk. rnk added a comment. @majnemer will be back Monday, grabbing this. https://reviews.llvm.org/D34714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D34714: [MS] Don't statically initialize dllimport member function pointers

2017-07-09 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL307446: [MS] Don't statically initialize dllimport member function pointers (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D34714?vs=104289&id=105768#toc Repository: rL LLVM ht

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

2017-07-11 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D35259#805284, @erichkeane wrote: > Oren discovered this miss to the original implementation. I'd reviewed this > internally quite a bit. > > The reason for the Win32ABI test is that MSVC 'long double' is actually small > enough for SSE register

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-07-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. > Some non-windows targets using MS extensions define _WIN32 for compatibility > with Windows but do not have MSVC compatibility. So, these hypothetical targets have the Win32 API available, but the

[PATCH] D35008: [AArch64] Produce the right kind of va_arg for windows

2017-07-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 Comment at: lib/CodeGen/TargetInfo.cpp:8463 Kind = AArch64ABIInfo::DarwinPCS; +else if (Triple.getOS() == llvm::Triple::Win32) + Kind = AArch64ABIInfo::Win64;

[PATCH] D34475: [AArch64] Add support for __builtin_ms_va_list on aarch64

2017-07-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: include/clang/Basic/BuiltinsAArch64.def:65 +// Win64-compatible va_list functions +BUILTIN(__builtin_ms_va_start, "vc*&.", "nt") +BUILTIN(__builtin_ms_va_end, "vc*&", "n") I strongly suspect that Microsoft will never adopt a

[PATCH] D34475: [AArch64] Add support for __builtin_ms_va_list on aarch64

2017-07-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: include/clang/Basic/BuiltinsAArch64.def:65 +// Win64-compatible va_list functions +BUILTIN(__builtin_ms_va_start, "vc*&.", "nt") +BUILTIN(__builtin_ms_va_end, "vc*&", "n") mstorsjo wrote: > rnk wrote: > > I strongly suspect

[PATCH] D35378: [clang] Add getSignedSizeType method

2017-07-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 Repository: rL LLVM https://reviews.llvm.org/D35378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D34475: [AArch64] Add support for __builtin_ms_va_list on aarch64

2017-07-17 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. Thanks, looks great! https://reviews.llvm.org/D34475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

2017-07-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:3516 + // calling convention is used. if (IsRegCall && FI.getReturnType()->getTypePtr()->isRecordType() && !FI.getReturnType()->getTypePtr()->isUnionType()) { This is incorrect. We shoul

[PATCH] D34873: Fix miscompiled 32bit binaries by mingw

2017-07-19 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. This doesn't seem like an acceptable workaround, surely this regresses functionality for arrays with more than UINT_MAX elements. https://reviews.llvm.org/D34873 __

[PATCH] D35546: [AArch64] Produce correct defaultlib directives for windows in MSVC style

2017-07-19 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/D35546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33852: Enable __declspec(selectany) on linux

2017-07-20 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: include/clang/Basic/Attr.td:2421 -def SelectAny : InheritableAttr, TargetSpecificAttr { +def SelectAny : InheritableAttr, TargetSpecificAttr { let Spellings = [Declspec<"selectany">, GCC<"selectany">]; Prazek wrote: > d

[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax

2017-07-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 Repository: rL LLVM https://reviews.llvm.org/D33277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

2017-07-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/D35259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35757: Work around an MSVC2017 update 3 codegen bug.

2017-07-24 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/D35757 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35903: [x86][inline-asm]Allow a pack of Control Regs to be properly picked

2017-07-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: rL LLVM https://reviews.llvm.org/D35903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D35941: Fix -Wshadow false positives with function-local classes.

2017-07-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I'm not really sure this is a bug. The point of -Wshadow is to warn on valid but possibly confusing code. Shadowing a variable is perfectly legal, but people think it's confusing, so we implemented this warning. Are we concerned that people might get confused between the di

[PATCH] D42092: implement C++ dr388 for the Itanium C++ ABI: proper handling of catching exceptions by reference

2018-01-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3867 +// subsequent catches in the same try statement to catch such a rethrow. +// See C++ DR 388. +if (!CanBindToTemporary && MaybeTypeMatch) { Based on our conversation, this fix f

[PATCH] D40925: Add option -fkeep-static-consts

2018-01-17 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. OK. My concern is that users need to use `__attribute__((used))` or something more robust if they want SVN identifiers to reliably appear in the output. Adding this flag just creates a trap that will fail once they turn on `-O2`. I'd rather not have it in the interface to a

[PATCH] D42208: Use an enum value instead of an string

2018-01-17 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/D42208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D42351: Emit DWARF "constructor" calling convention for every supported Clang CC

2018-01-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: aprantl. rnk added a comment. @aprantl Repository: rC Clang https://reviews.llvm.org/D42351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42455: Don't create hidden dllimport global values

2018-01-24 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/D42455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42641: [MinGW] Emit typeinfo locally for dllimported classes without key functions

2018-02-01 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. This is actually consistent with what Microsoft does for dllimport classes. They don't have key functions, but they do import vftables when a class is dllimport and the constructor is inline. They n

[PATCH] D42768: AST: add an extension to support SwiftCC on MS ABI

2018-02-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In https://reviews.llvm.org/D42768#994458, @rjmccall wrote: > No, I mean things like `void foo(__attribute__((swiftcall)) void > (*fnptr)());`. Yeah, this was the example I was going to bring up. There's no function parameter declaration to put the NNS on. So, here's an

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

2017-02-13 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL294978: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D29843?vs=88068&id=88229#toc Repository: rL LLVM h

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

2017-02-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I committed a modified patch that set the visibility on the global after creating it to avoid adding an optional boolean parameter. Thanks for the report and patch! Repository: rL LLVM https://reviews.llvm.org/D29843 ___ cf

[PATCH] D29912: [MS ABI] Correctly mangling vbase destructors

2017-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, thanks! https://reviews.llvm.org/D29912 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

2017-02-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. lgtm, do you need me to land this? Sorry I took a while to get around to this. https://reviews.llvm.org/D29464 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

2017-02-13 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. Actually, I re-read the concerns about omp.h. We should get that ironed out first. https://reviews.llvm.org/D29464 ___ cfe-commits mailing l

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

2017-02-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. If I understand correctly, these GCC version specific include directories are equivalent to clang's resource include directory. We shouldn't have those on the include search path, so in principle, this seems like the right change. https://reviews.llvm.org/D29464 ___

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

2017-02-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. So far as I can tell, Clang on Linux supports none of the headers under discussion here, and that's been OK for some time. None of the headers in question appear to actually work today, so I think we should remove them from the search path. Rega

[PATCH] D24812: Lit C++11 Compatibility Patch #11

2017-02-15 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: test/CodeGenCXX/static-init.cpp:14 +// CHECK98: @_ZZN5test414useStaticLocalEvE3obj = linkonce_odr global %"struct.test4::HasVTable" zeroinitializer, comdat, align 8 +// CHECK11: @_ZZN5test414useStaticLocalEvE3obj = linkonce_odr global { i8*

[PATCH] D30009: Add support for '#pragma clang attribute'

2017-02-16 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: docs/LanguageExtensions.rst:2349 +attribute is supported by the pragma by referring to the +:doc:`individual documentation for that attribute `. arphaman wrote: > efriedma wrote: > > arphaman wrote: > > > arphaman wrote: > >

[PATCH] D29772: Create msbuild only when using MSVC

2017-02-17 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/D29772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded

2017-02-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. My understanding is that Windows doesn't allow you to delete a file that has been opened and mapped into any process, even if it has been opened with FILE_SHARE_DELETE. One way to work around this would be to avoid memory mapping files in the SourceManager when using the Re

[PATCH] D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded

2017-02-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 We should file a bug against the Rewriter interface. It should really take MemoryBuffers from the caller and handle this for them. https://reviews.llvm.org/D30385 _

[PATCH] D30326: [MS-ABI] Allow #pragma section to choose for ZI data

2017-03-01 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. This functionality doesn't feel worth a driver flag. It customizes a very small aspect of a Microsoft extension that was only implemented for compatibility. It also reimplements the logic that LLVM uses in the backend to put global variables in .bss or .data. Can you elabor

[PATCH] D30518: Fix msc-version.c test to handle _MSC_VER=1910

2017-03-01 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/D30518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30015: Add arch-specific directory to search path

2017-03-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: include/clang/Driver/ToolChain.h:305 + // as OpenMP) to find arch-specific libraries. + const std::string getArchSpecificLibPath() const; + Why const qualify the std::string? Comment at: lib/Driver/Tools

[PATCH] D30590: Don't assume cleanup emission preserves dominance in expr evaluation

2017-03-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision. Because of the existence branches out of GNU statement expressions, it is possible that emitting cleanups for a full expression may cause the new insertion point to not be dominated by the result of the inner expression. Consider this example: struct Foo { Foo(); ~Foo

[PATCH] D30015: Add arch-specific directory to search path

2017-03-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. Looks good with a minor comment about a comment in the test case. Comment at: lib/Driver/Tools.cpp:2007-2009 + // In the cross-compilation case, arch-specific library path is likely + // unavailable at runtime. + if (TC.isCros

[PATCH] D30239: enable -flto=thin in clang-cl

2017-03-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. Do you guys think that maybe -flto should imply -fuse-ld=lld, or is that too much magic? Repository: rL LLVM https://reviews.llvm.org/D30239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D30590: Don't assume cleanup emission preserves dominance in expr evaluation

2017-03-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments. Comment at: lib/CodeGen/CGExprComplex.cpp:204-206 +Scope.ensureDominatingValue(&Vals.first); +Scope.ensureDominatingValue(&Vals.second); +Scope.ForceCleanup(); rsmith wrote: > I'm a little concerned about the loose connectio

[PATCH] D30590: Don't assume cleanup emission preserves dominance in expr evaluation

2017-03-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk updated this revision to Diff 90743. rnk added a comment. - comments https://reviews.llvm.org/D30590 Files: lib/CodeGen/CGCleanup.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprComplex.cpp lib/CodeGen/CGExprScalar.cpp lib/CodeGen/CodeGenFunction.h test/CodeGenCXX/stmtexpr.cpp Ind

[PATCH] D30662: Update clang filtering for mxcsr

2017-03-06 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I think we should really change this code to filter out clobber registers that do not name valid gcc inline asm clobbers. That's basically what we require later. I think if you change this condition to just check Target.isValidGCCRegisterName. Repository: rL LLVM https

<    13   14   15   16   17   18   19   20   >