[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

rnk added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4441-4462 + // Use global variables for enums in CodeView, use DW_TAG_enumeration_type for + // enums for non-CodeView. if (const auto *ECD = dyn_cast(VD)) { const auto *ED = cast(ECD->getDeclCont

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

rnk added a comment. I'm trying to figure out what exactly the new GlobalDestructor thing is. :) It's not clear to me why we ever emit `__cxx_global_array_dtor` in the MS ABI. We always call it directly from the `?__F` atexit helper stub instead of registering it, so we really don't need it at

[PATCH] D67141: [DebugInfo] Emit DW_TAG_enumeration_type for referenced global enumerator.

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

[PATCH] D66328: [DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm with the GlobalArrayDestructor name. Comment at: include/clang/AST/GlobalDecl.h:34 AtExit, + GlobalDestructor }; Maybe we should rename this GlobalArrayD

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk marked an inline comment as done. rnk added a comment. In D67028#1653439 , @efriedma wrote: > Do we have test coverage for a variadic, covariant thunk for a function > without a definition? I don't think there's any way for us to actually emit > tha

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk added a comment. I think what I said applies to your test case. Basically, in the Itanium C++ ABI, virtual method definitions provide all their thunks as `weak_odr`. We only emit thunks referenced by vtables as an optimization, and they are marked `available_externally`. In your test case,

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk updated this revision to Diff 218992. rnk added a comment. - merge into thunks.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 Files: clang/lib/CodeGen/CGVTables.cpp clang/test/CodeGenCXX/lineta

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk added a comment. In D67028#1659881 , @efriedma wrote: > > In your test case, we hit the early return that I linked to, so we don't > > try to clone, and we don't need to emit an error. > > Yes, that's what happens with the Itanium ABI; what happens wi

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk updated this revision to Diff 218995. rnk added a comment. - emit an error if we try to clone without a definition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67028/new/ https://reviews.llvm.org/D67028 Files: clang/lib/CodeGen/CGVTables.cp

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added a subscriber: hans. rnk added a comment. Here is some feedback, I apologize for dragging my feet. Also, I would really like to get feedback from @pcc. He's OOO currently, though. Comment at: clang/include/clang/Driver/Options.td:500 def b : JoinedOrSeparate<["-"], "

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

rnk added inline comments. Comment at: llvm/lib/Target/X86/X86FixupCFGuard.cpp:13 +/// for such cases and replaces the pair of instructions with a single +/// call/invoke. For example: +/// hans wrote: > Naive question: Why do we generate code as in the examples

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

rnk created this revision. rnk added reviewers: hans, rsmith, STL_MSFT. Herald added a project: clang. These three modes need to range check enumerator values differently than C++ does. Before this change, we had two codepaths doing the same thing in two cases: 1. enum complete (ObjC fixed & Micr

[PATCH] D67028: Use musttail for variadic method thunks when possible

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/test/CodeGenCXX/ms-thunks-variadic-return.cpp:9 +struct B : virtual A { + // expected-error@+1 2 {{cannot compile this return-adjusting thunk with variadic arguments yet}} + B *clone(const char

[PATCH] D67028: Use musttail for variadic method thunks when possible

This revision was automatically updated to reflect the committed changes. Closed by commit rL371269: Use musttail for variadic method thunks when possible (authored by rnk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

rnk updated this revision to Diff 219425. rnk added a comment. - rewrite, abandon unification Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67304/new/ https://reviews.llvm.org/D67304 Files: clang/lib/Sema/SemaDecl.cpp clang/test/Sema/Microsoft

[PATCH] D67304: Unify checking enumerator values in ObjC, C, and MSVC modes

rnk added a comment. In D67304#1663042 , @hans wrote: > Should there be a test exercising this part? The updated tests both have > -fms-compatibility, so were already just warning. Good point, we aren't testing that this unfixed enum behavior is a part

[PATCH] D67304: Emit -Wmicrosoft-enum-value warning instead of error in MS ABI

rnk requested review of this revision. rnk added a comment. Ptal, new patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67304/new/ https://reviews.llvm.org/D67304 ___ cfe-commits mailing list cfe-com

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

rnk added inline comments. Comment at: lib/AST/ExprConstant.cpp:583 +uint64_t& GetArrayInitIndex() { +return reinterpret_cast(ArrayInitIndex[0]); +} Surely this will fault on SPARC or ARM or other ISAs that care about alignment? https://reviews.

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

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. > Another data point is that GCC doesn't warn in this case. That seems like a reasonable tie breaker when implementing these kinds of style-enforcement warnings. :) https://reviews.llvm.org/D35941

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

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

[PATCH] D36100: [Attr] Make TargetWindows and TargetMicrosoftCXXABI match on aarch64 as well

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

[PATCH] D36098: [AArch64] Don't define __LP64__ when targeting Windows

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

[PATCH] D36105: [AArch64] Ignore stdcall and similar on aarch64/windows

rnk added inline comments. Comment at: lib/Basic/Targets/AArch64.cpp:466-467 + case CC_X86ThisCall: + case CC_X86FastCall: + case CC_X86VectorCall: +return CCCR_Ignore; Do they really ignore __fastcall and __vectorcall on arm64? I assume __thiscall and __

[PATCH] D36099: [test] Fix mistagged CHECK-NOT-lines for AARCH64-DARWIN in Preprocessor/init.c

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

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good, but let's confirm with Richard before we change behavior. https://reviews.llvm.org/D36238 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D36238: Use "foo-12345.o" instead of "foo.o-12345" as temporary file name.

rnk added a comment. I discovered this code in llvm/lib/Support/Windows/Signals.inc which explains why we leak so many temp files on Windows: // FIXME: open files cannot be deleted. if (FilesToRemove != NULL) while (!FilesToRemove->empty()) { llvm::sys::fs::remove(FilesToRemove->ba

[PATCH] D36230: [X86][Asm] Allow negative immediate to appear before bracketed expression

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

[PATCH] D36105: [AArch64] Ignore stdcall and similar on aarch64/windows

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

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

rnk added a comment. In https://reviews.llvm.org/D35056#834689, @rsmith wrote: > I also removed some incorrect assumptions from the Win64 ABI code; this > changed the behavior of one testcase from uncopyable-args.cpp > (`implicitly_deleted_copy_ctor::A` is now passed indirect). That's probabl

[PATCH] D36453: [libclang] Fix PR34055 (incompatible update of clang-c/Index.h)

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

[PATCH] D36437: [clang] Get rid of "%T" expansions

rnk accepted this revision. rnk added a comment. Thanks, most of these looks like potential races between tests waiting to happen. Repository: rL LLVM https://reviews.llvm.org/D36437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

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

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D35056#834705, @rnk wrote: > In https://reviews.llvm.org/D35056#834689, @rsmith wrote: > > > I also removed some incorrect assumptions from the Win64 ABI code; this > > c

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

rnk added inline comments. Comment at: test/CodeGenCXX/uncopyable-args.cpp:101 + +// In MSVC 2013, the copy ctor is not deleted by a move assignment. In MSVC 2015, it is. +// WIN64-18-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(i64 rsmith wrote

[PATCH] D36450: [X86][Ms-InlineAsm] Extend MS Dot operator to accept "this" + struct/class pointers aliases

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Comment at: lib/Sema/SemaStmtAsm.cpp:702-705 + // MS InlineAsm often uses struct pointer aliases as a base + const QualType QT = TD->getUnderlyingType(); + RT =

[PATCH] D36371: [Clang][x86][Inline Asm] support for GCC style inline asm - Y constraints

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

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

rnk added a subscriber: gbiv. rnk added a comment. I looked at the blame, and I added this alignment thing in https://reviews.llvm.org/rL289575 to deal with some PointerIntPair assertions. Those probably started in @gbiv's https://reviews.llvm.org/rL270781, which introduced a `PointerIntPair` f

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

rnk added a comment. This shouldn't be necessary after https://reviews.llvm.org/rL310905. https://reviews.llvm.org/D34873 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36474: Use the file name from linemarker for debug info if an input is preprocessed source.

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

[PATCH] D36931: Update LLVM 5.0 release notes for ms_abi and __builtin_ms_va_list for aarch64

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

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

rnk added a comment. > Don't add any EH edges to the CFG for SEH. In practice, BuildOpts.AddEHEdges > is always false in practice from what I can tell, and with SEH every single > stmt would have to get an EH edge. Since we can't mix C++ EH and SEH, do you think it would be better to reuse the

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

rnk added a comment. Looks functionally correct Comment at: test/Sema/warn-unreachable-ms.c:42 + } +} Can we add a test to exercise that this builds the right CFG? ``` __try { __try { f(); } __except(1) { __leave; // should exit outer try } __l

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Looks good! Comment at: test/Sema/warn-unreachable-ms.c:42 + } +} rnk wrote: > Can we add a test to exercise that this builds the right CFG? > ``` > __try { > _

[PATCH] D36914: Implement CFG construction for __try / __except / __leave.

rnk added inline comments. Comment at: test/Sema/warn-unreachable-ms.c:42 + } +} rnk wrote: > rnk wrote: > > Can we add a test to exercise that this builds the right CFG? > > ``` > > __try { > > __try { > > f(); > > } __except(1) { > > __leave; // sh

[PATCH] D36371: [Clang][x86][Inline Asm] support for GCC style inline asm - Y constraints

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

[PATCH] D36794: Fixups to FE tests affected by D36793

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. These all look good Repository: rL LLVM https://reviews.llvm.org/D36794 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D37090: Implement CFG construction for __finally.

rnk added a comment. Re: jumps out of __try, I wonder if you can tie __finally into whatever the CFG does for C++ destructors. Comment at: test/Sema/warn-unreachable-ms.c:49 __try { - f(); + throw 1; } __except (1) { Nice. Would any noreteu

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

rnk added inline comments. Comment at: lib/Lex/Lexer.cpp:3076-3077 case '\r': +if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r')) + Char = getAndAdvanceChar(CurPtr, Result); // If we are inside a preprocessor directive and we see the end of line,

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

rnk added inline comments. Comment at: lib/Lex/Lexer.cpp:3076-3077 case '\r': +if (CurPtr[0] != Char && (CurPtr[0] == '\n' || CurPtr[0] == '\r')) + Char = getAndAdvanceChar(CurPtr, Result); // If we are inside a preprocessor directive and we see the end of line,

[PATCH] D37079: [Preprocessor] Correct internal token parsing of newline characters in CRLF

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

[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime

rnk added a comment. In https://reviews.llvm.org/D37122#851978, @erichkeane wrote: > Ugg... disregard the system-header-line-directive-ms-lineendings.c issue, I'm > going to try to figure that out Yeah, I'm seeing issues with that as well. I'm not sure what's up. https://reviews.llvm.org

[PATCH] D71436: [X86][AsmParser] re-introduce 'offset' operator

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm, thanks! Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1821 +getParser().parsePrimaryExpr(Val, End)) + return Error(Start, "unexpected token!"); + }

[PATCH] D71439: Allow redeclaration of __declspec(uuid)

rnk added a comment. In D71439#1789141 , @zahen wrote: > @rnk Can you please submit on my behalf. I don't have commit access. Sorry for the delay, I pushed it today. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D71439: Allow redeclaration of __declspec(uuid)

This revision was automatically updated to reflect the committed changes. Closed by commit rG0acfc493171a: Allow redeclaration of __declspec(uuid) (authored by zahen, committed by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71439/new/ https:

[PATCH] D71991: Fix external-names.c test when separator is \\

rnk accepted this revision. rnk added a comment. lgtm, but why doesn't this show up on existing clang windows bots? For example: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/13276 IIRC clang often joins paths with `/`, so maybe this separator is always `/` upstream. Do you hav

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

rnk added subscribers: inglorion, gbiv. rnk added a comment. Herald added a subscriber: george.burgess.iv. +@gbiv @inglorion, other ThinLTO users. > To solve this situation, we capture (and retain) the initial intention until > the point of usage, through a new ThreadPoolStrategy class. The numb

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

rnk created this revision. rnk added reviewers: erichkeane, craig.topper. Herald added a project: clang. Before this change, X86_32ABIInfo::classifyArgument would be called twice on vector arguments to vectorcall functions. This function has side effects to track GPR register usage, and this would

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

rnk created this revision. rnk added reviewers: rjmccall, craig.topper, erichkeane. Herald added a project: clang. MSVC 2013 would refuse to pass highly aligned things (typically vectors and aggregates) by value. Users would receive this error: t.cpp(11) : error C2719: 'w': formal parameter wit

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1648 + // a bit different than the x64 version. First, all vector types (not HVAs) + // are assigned, with the first 6 ending up in the YMM0-5 or XMM0-5 registers. +

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

rnk marked 2 inline comments as done. rnk added inline comments. Comment at: clang/test/CodeGen/x86_32-arguments-win32.c:77 +// CHECK-LABEL: define dso_local void @receive_vec_256(<8 x float> inreg %x, <8 x float> inreg %y, <8 x float> inreg %z, <8 x float>* %0, <8 x float>* %1)

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

rnk updated this revision to Diff 236115. rnk added a comment. - ZYXMM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72110/new/ https://reviews.llvm.org/D72110 Files: clang/include/clang/CodeGen/CGFunctionInfo.h clang/lib/CodeGen/TargetInfo.cp

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

rnk marked 2 inline comments as done. rnk added inline comments. Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:91 bool InAllocaSRet : 1;// isInAlloca() + bool InAllocaIndirect : 1;// isInAlloca() bool IndirectByVal : 1; // isIndirect() rjm

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

rnk marked an inline comment as done. rnk added inline comments. Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:91 bool InAllocaSRet : 1;// isInAlloca() + bool InAllocaIndirect : 1;// isInAlloca() bool IndirectByVal : 1; // isIndirect() rnk

[PATCH] D69868: Allow "callbr" to return non-void values

rnk added subscribers: MaskRay, echristo. rnk added a comment. More reviewers: +@echristo @MaskRay Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:133 + /// Indicate that this basic block is the indirect dest of an INLINEASM_BR. + bool IsInlineAsmBrIndirectPad = fal

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers properly comdat

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm This will be a small change in behavior, but nobody on ELF should notice because things with vague linkage there are both ELF-weak and comdat. Comment at: clang/lib/CodeGen/

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

rnk added subscribers: aganea, mikerice. rnk added a comment. Honestly, MSVC's behavior makes more sense to me. Usually warnings tell the user they are doing something silly, and then let them do it anyway. Without this change, there is no way to add extra macros to some compilations, even if t

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

rnk added a comment. I'm interested in making clang do this, but I think this needs significantly more work until this is ready to land. It needs in-tree tests. I assumed we'd want to hook it up to `clang-cl /Yc` and `/Yu`. Comment at: clang/include/clang/Basic/LangOptions.de

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

rnk added a comment. > Total object file size on Windows, compiling with RelWithDebInfo: > > before: 4,257,448 kb > after: 2,104,963 kb > > And on Linux > > before: 9,225,140 kb > after: 4,387,464 kb These numbers are amazing! I made a summary of Amy's list of types that become incomplete

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

rnk added a comment. I think I liked the first version of this patch better. I would say, if the AST after instantiation remains the same as it was before D69950 , then the first one seems like the right fix. The pattern AST will just be missing a node to repres

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

rnk updated this revision to Diff 238147. rnk added a comment. - use %clang_cc1 as possible in tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72405/new/ https://reviews.llvm.org/D72405 Files: clang/lib/Lex/PPDirectives.cpp clang/test/PCH/

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. In D72405#1818237 , @zahen wrote: > I had no luck converting the CHECK-FOO & CHECK-NOFOO tests to use `-verify` > because the errors were reported against "(

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

This revision was automatically updated to reflect the committed changes. Closed by commit rG0f9cf42facaf: Allow /D flags absent during PCH creation under msvc-compat (authored by zahen, committed by rnk). Changed prior to commit: https://reviews.llvm.org/D72405?vs=238147&id=238149#toc Reposit

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

rnk added inline comments. Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:91 bool InAllocaSRet : 1;// isInAlloca() + bool InAllocaIndirect : 1;// isInAlloca() bool IndirectByVal : 1; // isIndirect() rjmccall wrote: > rnk wrote: > > rnk wrot

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

rnk updated this revision to Diff 238152. rnk marked 2 inline comments as done. rnk added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72114/new/ https://reviews.llvm.org/D72114 Files: clang/include/clang/CodeGen/CGFunctionI

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

This revision was automatically updated to reflect the committed changes. Closed by commit rG8e780252a728: [X86] ABI compat bugfix for MSVC vectorcall (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72110/new/ https://reviews.llvm.o

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

rnk added a comment. Thanks, the code change looks good, but please adjust the tests a bit. Comment at: clang/test/CodeGen/exceptions-seh-finally.c:284-286 -// Look for the absence of noinline. Enum attributes come first, so check that -// a string attribute is the first to ver

[PATCH] D72742: Don't assume promotable integers are zero/sign-extended already in x86-64 ABI.

rnk added subscribers: rjmccall, chandlerc. rnk added a comment. +@chandlerc @rjmccall Didn't we work this out already when John added the alignment tracking stuff? I remember this bug involving libjpegturbo standalone assembly receiving a 32-bit argument, and then using the full 64-bit RDI reg

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

rnk added a comment. I'm still in favor of this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67847/new/ https://reviews.llvm.org/D67847 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D72742: Don't assume promotable integers are zero/sign-extended already in x86-64 ABI.

rnk added a comment. In D72742#1822709 , @emilio wrote: > GCC does avoid the zero-extension on the caller sometimes, when not required > by the language and when its optimizer finds it suitable. I believe at the time, faced with ambiguity in the documen

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

rnk added a comment. In D87808#2280197 , @dblaikie wrote: > @rsmith What's the deal with these anonymous structs/unions? Why do they have > copy/move constructors (are those technically called from the enclosing > class's copy/move constructors?) but no

[PATCH] D87923: [MS] On x86_32, pass overaligned, non-copyable arguments indirectly

rnk created this revision. rnk added reviewers: aeubanks, craig.topper. Herald added a project: clang. rnk requested review of this revision. This updates the C++ ABI argument classification code to use the logic from D72114 , fixing an ABI incompatibility with MS

[PATCH] D88002: [clang-cl] Always interpret the LIB env var as separated with semicolons

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/D88002/new/ https://reviews.llvm.org/D88002 ___ cfe-com

[PATCH] D87923: [MS] On x86_32, pass overaligned, non-copyable arguments indirectly

This revision was automatically updated to reflect the committed changes. Closed by commit rG3b3a16548568: [MS] On x86_32, pass overaligned, non-copyable arguments indirectly (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87923/new/

[PATCH] D87888: [X86] Use inlineasm flag output for the _bittest* intrinsics.

rnk added a comment. Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D88005: [clang] [MinGW] Add an implicit .exe suffix even when crosscompiling

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/D88005/new/ https://reviews.llvm.org/D88005 ___ cfe-com

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

rnk added a comment. I think we can go forward with the reviewers we have. I have one more concern. Are the other reviewers happy? Comment at: clang/lib/Driver/Distro.cpp:206 +const llvm::Triple &TargetOrHost) { + static Distro::DistroType

[PATCH] D88121: [X86] Add a memory clobber to the bittest intrinsic inline asm. Get default clobbers from the target

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88121/new/ https://reviews.llvm.org/D88121 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

rnk accepted this revision. rnk added a comment. Looks good to me, I didn't review very in depth, but I see the test case that we need. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 __

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. `llvm::call_once` is fine, but IMO the static local version is preferable: it's built in to the language, no headers to include. Comment at: clang/lib/Driver/Distro.cpp:224 + if

[PATCH] D84362: [NFC] Refactor DiagnosticBuilder and PartialDiagnostic

rnk added subscribers: rjmccall, rnk. rnk added inline comments. Comment at: clang/include/clang/Basic/Diagnostic.h:1086-1090 /// Note that many of these will be created as temporary objects (many call /// sites), so we want them to be small and we never want their address take

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

rnk abandoned this revision. rnk added a comment. I actually forgot about this change entirely and uploaded and landed a new one: https://reviews.llvm.org/D87923 I suppose they are functionally different: if we have an overaligned type that can be passed in registers due to [[trivial_abi]], we s

[PATCH] D87888: [X86] Use inlineasm flag output for the _bittest* intrinsics.

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

[PATCH] D92883: De-templatify EmitCallArgs argument type checking, NFCI

rnk created this revision. rnk added reviewers: compnerd, aeubanks, rsmith. rnk requested review of this revision. Herald added a project: clang. This template exists to abstract over FunctionPrototype and ObjCMethodDecl, which have similar APIs for storing parameter types. In place of a template,

[PATCH] D92944: Don't setup inalloca for swiftcc on i686-windows-msvc

rnk created this revision. rnk added reviewers: compnerd, rjmccall. rnk requested review of this revision. Herald added a project: clang. Swiftcall does it's own target-independent argument type classification, since it is not designed to be ABI compatible with anything local on the target that is

[PATCH] D92883: De-templatify EmitCallArgs argument type checking, NFCI

rnk updated this revision to Diff 310569. rnk added a comment. - use assign instead of push_back Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92883/new/ https://reviews.llvm.org/D92883 Files: clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGE

[PATCH] D91673: [PGO] Enable preinline and cleanup when optimize for size

rnk added a comment. I see two pre-commit test failures on the patch. Can you take a look at those as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91673/new/ https://reviews.llvm.org/D91673 ___ c

[PATCH] D92883: De-templatify EmitCallArgs argument type checking, NFCI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd7098ff29c58: De-templatify EmitCallArgs argument type checking, NFCI (authored by rnk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D92944: Don't setup inalloca for swiftcc on i686-windows-msvc

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdf282215d497: Don't setup inalloca for swiftcc on i686-windows-msvc (authored by rnk). Changed prior to commit: https://reviews.llvm.org/D92944?vs

[PATCH] D92800: [Clang] Make nomerge attribute a function attribute as well as a statement attribute.

rnk added a comment. Nice, that wasn't too difficult. I had some suggestions for improving the test case, and I'd like to hear from Aaron. Comment at: clang/test/CodeGen/attr-nomerge.cpp:8 + [[clang::nomerge]] void f(); + [[clang::nomerge]] virtual void g(); + [[clang::nome

[PATCH] D92800: [Clang] Make nomerge attribute a function attribute as well as a statement attribute.

rnk added inline comments. Comment at: clang/test/CodeGen/attr-nomerge.cpp:8 + [[clang::nomerge]] void f(); + [[clang::nomerge]] virtual void g(); + [[clang::nomerge]] static void f1(); zequanwu wrote: > rnk wrote: > > Hm, virtual functions, there's something

[PATCH] D43002: [CodeView] Emit S_OBJNAME record

rnk added a comment. I think the biggest concern here is what to do about `/Fa` / `-save-temps`. If we do nothing, the S_OBJNAME record will change if you run the same compilation twice with and without those flags. Generally, we strive to ensure that the directly emitted object file is identic

<    5   6   7   8   9   10   11   12   13   14   >