[PATCH] D50199: [MinGW] Predefine UNICODE if -municode is specified during compilation

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

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

rnk added a comment. Neat, thanks for the optimization. My only concern would be to find a way to avoid including Type.h, but it's already a very popular and very necessary header, so I don't think there is any issue here. Repository: rL LLVM https://reviews.llvm.org/D50261 _

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

rnk added a comment. In https://reviews.llvm.org/D50055#1188479, @chandlerc wrote: > Maybe double check with Reid and/or Hal to make sure we've all ended up on > the same page before landing though. lgtm, thanks https://reviews.llvm.org/D50055 _

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

rnk added a subscriber: beanz. rnk added a comment. In https://reviews.llvm.org/D15225#1191304, @george.karpenkov wrote: > @rnk As discussed, would it be acceptable for you to just have empty > sanitizer runtime files in the resource directory? I was talking to @beanz, and he suggested adding

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

rnk added a comment. I got some actual data. The way we package clang today, the extracted installation is 134.83M, and lib/clang/7.0.0/lib/darwin/* is 13M, so it's a 10% increase. It would be worth it to us to replace these with empty files if this change does land, but again, I'd rather not g

[PATCH] D50513: clang-cl: Support /guard:cf,nochecks

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! There is some additional work to do to put .gfids$y sections into appropriate comdat sections, but that's future work. https://reviews.llvm.org/D50513

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

rnk added a comment. In https://reviews.llvm.org/D49240#1195237, @ldionne wrote: > In https://reviews.llvm.org/D49240#1195125, @thakis wrote: > > > When we updated out clang bundle in chromium (which includes libc++ > > headers), our ios simulator bots regressed debug info size by ~50% due to >

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

rnk added a comment. In https://reviews.llvm.org/D49240#1195733, @ldionne wrote: > Ah, thanks a lot for taking a look! Yes, this makes a lot of sense, since now > we're not inlining everything anymore. So the code size is actually smaller > (-9.8%), but there's more symbols because more functio

[PATCH] D50564: Add support for SEH unwinding on Windows.

rnk added inline comments. Comment at: src/libunwind_ext.h:43 + #if defined(__x86_64__) && !defined(__MINGW64__) +typedef struct _DISPATCHER_CONTEXT { + ULONG64 ControlPc; cdavis5x wrote: > mstorsjo wrote: > > What's this all about? winnt.h (from both MSVC and

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

rnk added a comment. I'd prefer not to do this, since internal_linkage generates smaller, more debuggable code by default. I think the symbol table size increase may be specific to MachO, and it may be possible to work around this by changing ld64 to pool strings for symbols by default. I don't

[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

rnk added a comment. In https://reviews.llvm.org/D50652#1197780, @ldionne wrote: > One thing to keep in mind is that we do not have a solution that allows > removing both `internal_linkage` and `always_inline`. It's either > `internal_linkage` or `always_inline`, but you can't get rid of both u

[PATCH] D50547: [Driver] Use normalized triples for multiarch runtime path

rnk added a comment. Would it be to much to check both the normalized and non-normalized? Repository: rC Clang https://reviews.llvm.org/D50547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D50678: [InlineAsm] Update the min-legal-vector-width function attribute based on inputs and outputs to inline assembly

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

[PATCH] D50526: Model type attributes as regular Attrs

rnk added a comment. I suspect that somehow this change broke compiling ATL: https://ci.chromium.org/buildbot/chromium.clang/ToTWin64%28dbg%29/993 https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTWin64_dbg_%2F995%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Flogs%2Fraw_io.output_failur

[PATCH] D50526: Model type attributes as regular Attrs

rnk added a comment. I went ahead and reverted this in https://reviews.llvm.org/rC339638, and will produce a reduction soon. Repository: rC Clang https://reviews.llvm.org/D50526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D50526: Model type attributes as regular Attrs

rnk added a comment. The issue was related to ignored calling convention attributes on Win64: int a(int, const int *, int, int, __int64); class b { public: typedef int c; }; template void AtlAxDialogCreateT(int, d, int, int, __int64); int g, i, j, k; char *h; void l() { Atl

[PATCH] D50564: Add support for SEH unwinding on Windows.

rnk added a comment. In https://reviews.llvm.org/D50564#1198996, @cdavis5x wrote: > Could somebody verify that the `DISPATCHER_CONTEXT` struct is defined in > `` for the Win8 and Win10 SDKs? I can't install them right now. I checked both, and they are available, so I think we should guard the

[PATCH] D50805: Don't warn on returning the address of a label

rnk created this revision. rnk added reviewers: niravd, rsmith, nickdesaulniers. There aren't any lifetime issues inherent in returning a local label. Hypothetically, it could be used to drive a state machine driven by computed goto the next time that same scope is re-entered. In any case, the Lin

[PATCH] D50805: Don't warn on returning the address of a label

rnk added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7872-7874 -def warn_ret_addr_label : Warning< - "returning address of label, which is local">, - InGroup; lebedev.ri wrote: > Why completely drop the diagnostic just because

[PATCH] D50805: Don't warn on returning the address of a label

rnk added a comment. In https://reviews.llvm.org/D50805#1201805, @rsmith wrote: > There is no guarantee that you can use an address-of-label value from one > function invocation in another invocation of the same function. GCC's > documentation says it's the user's own problem to prevent inlinin

[PATCH] D50805: [Sema] Don't warn on returning the address of a label

rnk added a comment. Sounds good. I think, in the meantime, we all agree we can stop emitting this warning in the statement expression case. I'll upload a patch for that. Repository: rC Clang https://reviews.llvm.org/D50805 ___ cfe-commits maili

[PATCH] D50805: [Sema] Don't warn on returning the address of a label

rnk updated this revision to Diff 161096. rnk added a comment. - keep the warning, suppress stmt expr case https://reviews.llvm.org/D50805 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/statements.c Index: clang/test/Sema/statements.c ==

[PATCH] D50805: Don't warn on returning the address of a label from a statement expression

rnk updated this revision to Diff 161098. rnk added a comment. - fix it right https://reviews.llvm.org/D50805 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/statements.c Index: clang/test/Sema/statements.c === --- clang/t

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

rnk created this revision. rnk added reviewers: majnemer, inglorion, hans. Herald added subscribers: dexonsmith, JDevlieghere, aprantl, mehdi_amini. This is needed to avoid conflicts in mangled names for codeview types in anonymous namespaces. In CodeView, types refer to each other typically throu

[PATCH] D50805: Don't warn on returning the address of a label from a statement expression

rnk updated this revision to Diff 161148. rnk added a comment. - return to avoid bad notes https://reviews.llvm.org/D50805 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/statements.c Index: clang/test/Sema/statements.c ==

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

rnk updated this revision to Diff 161293. rnk added a comment. - Use xxHash64 https://reviews.llvm.org/D50877 Files: clang/lib/AST/MicrosoftMangle.cpp clang/test/CodeGenCXX/cfi-cross-dso.cpp clang/test/CodeGenCXX/cfi-icall.cpp clang/test/CodeGenCXX/debug-info-thunk.cpp clang/test/Code

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

rnk marked an inline comment as done. rnk added a comment. Exactly, this makes our names match MSVC more closely. Their hash depends on the path to the main source file. It doesn't care if the file is in a header. However, they use the absolute path to the file instead of the (probably relative

[PATCH] D50907: Make __shiftleft128 / __shiftright128 real compiler built-ins.

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

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

rnk updated this revision to Diff 161310. rnk marked an inline comment as done. rnk added a comment. - improve comment https://reviews.llvm.org/D50877 Files: clang/lib/AST/MicrosoftMangle.cpp clang/test/CodeGenCXX/cfi-cross-dso.cpp clang/test/CodeGenCXX/cfi-icall.cpp clang/test/CodeGenC

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

rnk added a comment. In https://reviews.llvm.org/D50877#1204609, @thakis wrote: > Can you explicitly mention that this intentionally doesn't use an absolute > path in MicrosoftMangleContextImpl() or similar? Sure. I also described the issue with codeview that motivates why we want unique name

[PATCH] D50877: [MS] Mangle a hash of the main file path into anonymous namespaces

This revision was automatically updated to reflect the committed changes. Closed by commit rC340079: [MS] Mangle a hash of the main file path into anonymous namespaces (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D50877?vs=161310&id=161323#toc Repository:

[PATCH] D50924: [CodeGen] add rotate builtins

rnk added a comment. Do you mind updating the _rotl* and _rotr* intrinsics to use the same codegen? They're right above in the switch. https://reviews.llvm.org/D50924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[PATCH] D50805: Don't warn on returning the address of a label from a statement expression

This revision was automatically updated to reflect the committed changes. Closed by commit rC340101: Don't warn on returning the address of a label from a statement expression (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D50805?vs=161148&id=161344#toc Rep

[PATCH] D50924: [CodeGen] add rotate builtins

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

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

rnk added inline comments. Comment at: include/clang/Basic/LangOptions.def:311 +BENIGN_LANGOPT(KeepStaticConsts , 1, 0, "keep static const variables even if unused") + Let's make this a CodeGenOption, since only CodeGen needs to look at it. https://revi

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

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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

rnk added a comment. Remind me what the approximate size overhead of this is? I expect it is negligible, as most symbols are not address taken. Repository: rL LLVM https://reviews.llvm.org/D51049 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D51026: [CodeGen] Implicitly set stackrealign on the main function, if custom stack alignment is used

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/CodeGen/CodeGenFunction.cpp:989 +CGM.getCodeGenOpts().StackAlignment) + Fn->addFnAttr("stackrealign"); + mstorsjo wrote: > erichk

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

rnk added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:1276 +InitVar->setSection(".CRT$XCLa"); +CGM.addUsedGlobal(InitVar); + } mstorsjo wrote: > rjmccall wrote: > > DHowett-MSFT wrote: > > > rjmccall wrote: > > > > Is the priority system not go

[PATCH] D46320: Remove \brief commands from doxygen comments.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm! (Back from a week long vacation) https://reviews.llvm.org/D46320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D46332: [X86] Only enable the __ud2 and __int2c builtins if intrin.h has been included.

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

[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.

rnk added inline comments. Comment at: include/clang/Basic/Attr.td:1494 +def NoStackProtector : InheritableAttr { + let Spellings = [GCC<"no_stack_protector">]; + let Subjects = SubjectList<[Function]>; aaron.ballman wrote: > manojgupta wrote: > > aaron.ballman

[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode

rnk reopened this revision. rnk added a comment. This revision is now accepted and ready to land. Please don't do this, this is actually really problematic, since `#line` directives lose information about what's a system header. That means the result of -E usually won't compile, since Windows he

[PATCH] D43320: Allow dllimport non-type template arguments in C++17

rnk updated this revision to Diff 146025. rnk added a comment. - don't expose CCEK, use a bool https://reviews.llvm.org/D43320 Files: clang/include/clang/AST/Expr.h clang/lib/AST/ExprConstant.cpp clang/lib/Sema/SemaOverload.cpp clang/test/SemaCXX/dllimport-constexpr.cpp clang/test/Sem

[PATCH] D43320: Allow dllimport non-type template arguments in C++17

rnk added inline comments. Comment at: clang/include/clang/AST/Expr.h:670-672 + /// Evaluate an expression that is required to be a core constant expression. + bool EvaluateAsCoreConstExpr(EvalResult &Result, QualType ParamType, + CCEKind CCE, cons

[PATCH] D43320: Allow dllimport non-type template arguments in C++17

rnk updated this revision to Diff 146039. rnk added a comment. - getting closer https://reviews.llvm.org/D43320 Files: clang/include/clang/AST/Expr.h clang/lib/AST/ExprConstant.cpp clang/lib/Sema/SemaOverload.cpp clang/test/SemaCXX/dllimport-constexpr.cpp clang/test/SemaCXX/dllimport-

[PATCH] D43320: Allow dllimport non-type template arguments in C++17

rnk added inline comments. Comment at: clang/include/clang/AST/Expr.h:662 + /// Indicates how the constant expression will be used. + enum ConstExprUsage { EvaluateForCodeGen, EvaluateForMangling }; + I expect we could come up with a better name, but is this cl

[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()

rnk added a comment. Is it possible to fix this in assignInheritanceModel instead? I'd imagine we'd get the most recent decl. If that's not the issue, maybe you're fixing the bug in the right spot, but we need to find out where other class template attributes are moved from instantiation to rea

[PATCH] D43320: Allow dllimport non-type template arguments in C++17

This revision was automatically updated to reflect the committed changes. Closed by commit rC332018: Allow dllimport non-type template arguments in C++17 (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D43320?vs=146039&id=146178#toc Repository: rC Clang h

[PATCH] D46520: [Driver] Use -fuse-line-directives by default in MSVC mode

rnk added a comment. In https://reviews.llvm.org/D46520#1092681, @mstorsjo wrote: > Reverted in SVN r331858. Thanks! When one attempts to generate pre-processed source with clang and compile it with MSVC, you usually have to remove clang's internal intrinsic headers from the include search pa

[PATCH] D43320: Allow dllimport non-type template arguments in C++17

rnk added a comment. Thanks for the guidance! Comment at: clang/lib/AST/ExprConstant.cpp:1871-1902 if (!CheckConstantExpression(Info, DiagLoc, EltTy, Value.getArrayInitializedElt(I))) return false; } if (!Value.hasAr

[PATCH] D46656: [Builtins] Improve the IR emitted for MSVC compatible rotr/rotl builtins to match what the middle and backends understand

rnk added a comment. Thanks! Repository: rL LLVM https://reviews.llvm.org/D46656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

rnk added a comment. I searched around, and I noticed that `VTableContext::getMethodVTableIndex` has the same implied contract that the caller must always provide a canonical declaration or things will break. It looks like it has three callers, and they all do this. We should probably sink the

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

rnk added a comment. @hans @thakis, do you remember how these flags are supposed to work? I've forgotten anything I ever knew about them... https://reviews.llvm.org/D46652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Shall I go ahead and commit this for you? Comment at: lib/AST/VTableBuilder.cpp:2507 const MethodInfo &MI = I.second; + assert(MD == MD->getCanonicalDecl()); + --

[PATCH] D46929: Fix a mangling failure on clang-cl C++17

This revision was automatically updated to reflect the committed changes. Closed by commit rC332639: Fix a mangling failure on clang-cl C++17 (authored by rnk, committed by ). Changed prior to commit: https://reviews.llvm.org/D46929?vs=147244&id=147359#toc Repository: rC Clang https://revie

[PATCH] D46910: [Support] Avoid normalization in sys::getDefaultTargetTriple

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

[PATCH] D47142: [x86] invpcid intrinsic

rnk added a comment. Why do we need a feature flag for this in the first place? The MSVC model for most "instruction" intrinsics is that the exact instruction is emitted regardless of the feature enabled. The target attribute seems like it would get in the way of that. Comme

[PATCH] D47142: [x86] invpcid intrinsic

rnk added inline comments. Comment at: lib/Headers/intrin.h:196 + */ void __cdecl _invpcid(unsigned int, void *); +#endif GBuella wrote: > GBuella wrote: > > rnk wrote: > > > craig.topper wrote: > > > > @rnk, what's the right thing to do here? > > > What problem

[PATCH] D47182: [X86] Move all Intel defined intrinsic includes into immintrin.h

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D47182#1107900, @craig.topper wrote: > Eventually this was determined to not be very scalable to remember which > header file contained what intrinsics and you have to ch

[PATCH] D47174: [X86] Move 128-bit f16c intrinsics to __emmintrin_f16c.h include from emmintrin.h. Move 256-bit f16c intrinsics back to f16cintrin.h

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

[PATCH] D47357: [Driver] Rename DefaultTargetTriple to TargetTriple

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

[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types

rnk added a comment. I think the approach makes sense. Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460 CGObjCNonFragileABIMac::GetEHType(QualType T) { // There's a particular fixed type info for 'id'. if (T->isObjCIdType() || T->isObjCQualifiedIdType()) { +if (CGM.ge

[PATCH] D47503: Sema: Add a warning for member pointers with incomplete base types.

rnk added a comment. Should we do this in exactly the places we would lock in an inheritance model in the MS ABI, i.e. when the member pointer type is required to be complete? I think we could take this code: bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,

[PATCH] D38226: [XRay][Driver] Do not link in XRay runtime in shared libs

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

[PATCH] D38158: [Sema] Null check in BuildDeclarationNameExpr

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/D38158 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D38303: [Sema] Correct IUnknown to support Unknwnbase.h Header.

rnk added inline comments. Comment at: lib/AST/DeclCXX.cpp:1497-1505 + bool IsInNamespace = false; + const auto *DeclContext = getDeclContext(); + while (!DeclContext->isTranslationUnit()) { +if (DeclContext->isNamespace()) { + IsInNamespace = true; + break; +

[PATCH] D38303: [Sema] Correct IUnknown to support Unknwnbase.h Header.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/AST/DeclCXX.cpp:1473 +static bool IsDeclContextInNamespace(const DeclContext *DC) { + while (!DC->isTranslationUnit()) { Lower case `isDecl

[PATCH] D37466: D37461: fixups for existing InlineAsm tests + adding new ones

rnk added inline comments. Comment at: test/CodeGen/ms-inline-asm.cpp:37-38 - int lvar = 10; - __asm mov eax, offset Foo::ptr - __asm mov eax, offset Foo::Bar::ptr -// CHECK-LABEL: define void @_Z2t2v() coby wrote: > rnk wrote: > > These don't seem tested anyw

[PATCH] D38290: Add a ld64.lld alias for the MACHO LLD target

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D38290#885503, @ruiu wrote: > This patch virtually sets `ld64` the linker command name for macOS. I'd be a > bit reluctant doing that, because `ld64` sounds like a too ge

[PATCH] D38123: [driver] [cl] Add/fix c++17/c++latest

rnk accepted this revision. rnk added a comment. Sorry for the delay, thank you, this looks good! @martell, do you mind landing this? https://reviews.llvm.org/D38123 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

rnk added inline comments. Comment at: lib/Basic/TargetInfo.cpp:29 +namespace { +TargetInfo::IntType GetDefaultWCharType(const llvm::Triple &T) { + const llvm::Triple::ArchType Arch = T.getArch(); How is this better than what we had before? It's totally inconsis

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

rnk added inline comments. Comment at: lib/Basic/TargetInfo.cpp:29 +namespace { +TargetInfo::IntType GetDefaultWCharType(const llvm::Triple &T) { + const llvm::Triple::ArchType Arch = T.getArch(); compnerd wrote: > rnk wrote: > > How is this better than what we

[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017

rnk created this revision. This raises our default past 1900, which controls whether char16_t is a builtin type or not. Implements PR34243 https://reviews.llvm.org/D38646 Files: clang/lib/Driver/ToolChains/MSVC.cpp Index: clang/lib/Driver/ToolChains/MSVC.cpp ===

[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017

rnk updated this revision to Diff 118079. rnk added a comment. - go to 1911, 1910 was the preview - add release notes https://reviews.llvm.org/D38646 Files: clang/docs/ReleaseNotes.rst clang/lib/Driver/ToolChains/MSVC.cpp Index: clang/lib/Driver/ToolChains/MSVC.cpp ===

[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017

This revision was automatically updated to reflect the committed changes. Closed by commit rL315107: [MS] Raise the default value of _MSC_VER to 1911, which is VS 2017 (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D38646?vs=118079&id=118080#toc Repository: rL LLVM htt

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good with nits Comment at: lib/Basic/Targets/AArch64.cpp:47-51 + bool IsNetBSD = getTriple().getOS() == llvm::Triple::NetBSD; + bool IsOpenBSD = getTriple().getOS() == llvm

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

rnk added inline comments. Comment at: src/AddressSpace.hpp:388-389 + found_obj = true; + } else if (!strncmp((const char *)pish->Name, ".eh_frame", + IMAGE_SIZEOF_SHORT_NAME)) { +info.dwarf_section = begin; ".eh_fra

[PATCH] D38646: [MS] Raise the default value of _MSC_VER to 1910, which is in VS 2017

rnk added a comment. In https://reviews.llvm.org/D38646#892246, @STL_MSFT wrote: > FYI: 1910 was the value for VS 2017 RTM. 1911 is the value for VS 2017 15.3, > the first toolset update. 1912 will be the value for VS 2017 15.5, the second > toolset update. Yep. The initial draft of this patc

[PATCH] D38679: [libunwind] Support dwarf unwinding on i386 windows

rnk added inline comments. Comment at: src/AddressSpace.hpp:388-389 + found_obj = true; + } else if (!strncmp((const char *)pish->Name, ".eh_frame", + IMAGE_SIZEOF_SHORT_NAME)) { +info.dwarf_section = begin; mstorsjo

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

rnk added inline comments. Comment at: src/libunwind.cpp:188 + co->getInfo(&info); + pint_t orgArgSize = (pint_t)info.gp; + uint64_t orgFuncStart = info.start_ip; I think it makes sense to have this here: the contract is that if the personality se

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

rnk accepted this revision. rnk added a comment. lgtm Comment at: src/libunwind.cpp:188 + co->getInfo(&info); + pint_t orgArgSize = (pint_t)info.gp; + uint64_t orgFuncStart = info.start_ip; mstorsjo wrote: > rnk wrote: > > I think it makes sense

[PATCH] D38700: [Sema][Crash] Correctly handle an non-dependent noexcept expr in function template

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

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

rnk added a comment. In https://reviews.llvm.org/D39079#904050, @lebedev.ri wrote: > No tests? +1, there should be an -emit-llvm test in clang/test/CodeGen/. Comment at: lib/CodeGen/CGCall.cpp:1859 +if (auto *Fn = dyn_cast(TargetDecl)) { + if (!Fn->isDefined() && !A

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

rnk added inline comments. Comment at: src/UnwindRegistersRestore.S:98 + # skip fs + # skip gs + movq 56(%rcx), %rsp # cut back rsp to new location mstorsjo wrote: > mstorsjo wrote: > > compnerd wrote: > > > Doesn't Win64 ABI require some of the MMX register

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

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

[PATCH] D39206: [libunwind] Add missing checks for register number

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

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

rnk added inline comments. Comment at: lib/Sema/SemaType.cpp:3269-3273 + bool IsMain = false; + if (D.getIdentifier() && D.getIdentifier()->isStr("main") && + S.CurContext->getRedeclContext()->isTranslationUnit() && + !S.getLangOpts().Freestanding) +IsMain = true;

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

rnk added inline comments. Comment at: lib/Sema/SemaType.cpp:3269-3273 + bool IsMain = false; + if (D.getIdentifier() && D.getIdentifier()->isStr("main") && + S.CurContext->getRedeclContext()->isTranslationUnit() && + !S.getLangOpts().Freestanding) +IsMain = true;

[PATCH] D39064: implement __has_unique_object_representations

rnk added inline comments. Comment at: lib/AST/Type.cpp:2226 +Context.getFieldOffset(*Record->field_begin())); +for (const auto *Field : Record->fields()) { + if (!Field->getType().hasUniqueObjectRepresentations(Context)) What about base classes?

[PATCH] D39127: Fix template parameter default args missed if redecled

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

[PATCH] D39127: Fix template parameter default args missed if redecled

rnk added a comment. I think you forgot to svn add the test case Repository: rL LLVM https://reviews.llvm.org/D39127 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

rnk added a comment. In https://reviews.llvm.org/D39079#905372, @joerg wrote: > Why again is this a good idea? It saves the direct jump to the PLT, reducing icache pressure, which is a major cost in some workloads. > This is an even worse hack than -Bsymbolic, Personally, I would like to bui

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

rnk added a comment. In https://reviews.llvm.org/D39079#905395, @joerg wrote: > Let me phrase it differently. What is this patch (and the matching backend > PR) supposed to achieve? There are effectively two ways to get rid of PLT > entries: > (1) Bind references locally. This is effectively w

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

rnk added a comment. Can you remind me why `NamedDecl::printQualifiedName` is not appropriate for your needs? https://reviews.llvm.org/D39224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

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

rnk added a comment. In https://reviews.llvm.org/D39079#905454, @joerg wrote: > It also increases the pressure on the branch predictor, so it is not really > black and white. I don't understand this objection. I'm assuming that the PLT stub is an indirect jump through the PLTGOT, not a hotpat

[PATCH] D39064: implement __has_unique_object_representations

rnk added inline comments. Comment at: lib/AST/Type.cpp:2212-2213 +for (const auto Base : ClassDecl->bases()) { + if (Base.isVirtual()) +return false; + OK, I guess vbases don't have a unique representation. :) What about vtable slots? Should we

[PATCH] D39064: implement __has_unique_object_representations

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

[PATCH] D51049: Driver: Enable address-significance tables by default when targeting COFF.

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

[PATCH] D51157: [x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative Load Hardening.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. It makes sense to me to do this all in one go and defer the clang `__attribute__` until later. Comment at: llvm/include/llvm/IR/Attributes.td:249 def : MergeRule<"adjustNullPoint

  1   2   3   4   5   6   7   8   9   10   >