Re: [PATCH] D10881: [Sema] Catch a case when 'volatile' qualifier is dropped while binding

rjmccall added a comment. This is outside of my expertise, but I've asked Doug Gregor to take a look. http://reviews.llvm.org/D10881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

rjmccall added a comment. Yes, the main result is defined to be the true value mod 2^k even when overflow occurs. We do use the intrinsics to implement the existing fixed-width GCC builtins, which are meant to be useful not just for security checking, but for implementing arbitrary-precision a

r248436 - Forbid qualifiers on ObjC generic parameters and arguments, but

Author: rjmccall Date: Wed Sep 23 17:14:21 2015 New Revision: 248436 URL: http://llvm.org/viewvc/llvm-project?rev=248436&view=rev Log: Forbid qualifiers on ObjC generic parameters and arguments, but silently ignore them on arguments when they're provided indirectly (.e.g behind a template argument

Re: [PATCH] D12793: Three new security overflow builtins with generic argument types

rjmccall added a comment. X and Y aren't unreasonable for the operands, although you could also use "left" and "right" (or LHS/RHS), especially since it's significant for subtraction. R is short for "result" and should be spelled out. E is presumably short for "encompassing", but that is not

Re: r246985 - Compute and preserve alignment more faithfully in IR-generation.

> On Sep 27, 2015, at 5:56 PM, Joerg Sonnenberger > wrote: > On Tue, Sep 08, 2015 at 08:06:00AM -0000, John McCall via cfe-commits wrote: >> Author: rjmccall >> Date: Tue Sep 8 03:05:57 2015 >> New Revision: 246985 >> >> URL: http://llvm.org/viewvc/

Re: [PATCH] D12793: Three new overflow builtins with generic argument types

rjmccall added a comment. LGTM, thanks. http://reviews.llvm.org/D12793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r248775 - Honor the casted-to alignment of an explicit cast even when

Author: rjmccall Date: Mon Sep 28 23:37:40 2015 New Revision: 248775 URL: http://llvm.org/viewvc/llvm-project?rev=248775&view=rev Log: Honor the casted-to alignment of an explicit cast even when Sema thinks the cast is a no-op, as it does when (e.g.) the only thing that changes is an alignment att

r248862 - Don't crash when a reserved global placement operator new is paired

Author: rjmccall Date: Tue Sep 29 18:55:17 2015 New Revision: 248862 URL: http://llvm.org/viewvc/llvm-project?rev=248862&view=rev Log: Don't crash when a reserved global placement operator new is paired with a non-reserved operator delete in a new-expression. Modified: cfe/trunk/lib/CodeGen/C

Re: r246985 - Compute and preserve alignment more faithfully in IR-generation.

> On Sep 27, 2015, at 5:56 PM, Joerg Sonnenberger > wrote: > On Tue, Sep 08, 2015 at 08:06:00AM -0000, John McCall via cfe-commits wrote: >> Author: rjmccall >> Date: Tue Sep 8 03:05:57 2015 >> New Revision: 246985 >> >> URL: http://llvm.org/viewvc/

cfe-commits@lists.llvm.org

rjmccall added a comment. LGTM. http://reviews.llvm.org/D13325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

rjmccall added a comment. You only have one attempt to define the function here; I don't see the problem. Recall that I said to add a flag to getOrCreateLLVMFunction that says whether the caller intends to define the function. The rule should be that only callers that pass "true" should be al

Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

rjmccall added a comment. In http://reviews.llvm.org/D11297#223622, @andreybokhanko wrote: > John, > > Thank you for the quick reply! > > Let me make sure I understand what you said, using my test as an example > (BTW, sorry if this is a dumb question -- I asked our local Clang experts, > but n

Re: r243851 - Fix invalid shufflevector operands

> On Aug 10, 2015, at 12:40 PM, Hans Wennborg wrote: > On Fri, Aug 7, 2015 at 11:57 AM, Simon Pilgrim wrote: >> On 06/08/2015 18:05, Hans Wennborg wrote: >>> >>> On Sun, Aug 2, 2015 at 8:28 AM, Simon Pilgrim >>> wrote: Author: rksimon Date: Sun Aug 2 10:28:10 2015 New Revi

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added a comment. Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate patch? Comment at: lib/CodeGen/CGCXXABI.h:349-357 @@ -348,1 +348,11 @@ + /// Checks if ABI requires extra virtual offset for vtable field. + virtual bool + isVirtualOffsetNeed

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added a comment. In http://reviews.llvm.org/D11859#225384, @Prazek wrote: > In http://reviews.llvm.org/D11859#225025, @rjmccall wrote: > > > Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate > > patch? > > > I wasn't planning to. I am focusing now on upgrading GVN

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1862 @@ +1861,3 @@ + for (const VPtr &Vptr : getVTablePointers(ClassDecl)) +if (CGM.getCXXABI().requiresVPtrInitialization(Vptr)) + EmitVTableAssumptionLoad(Vptr, This); No, it only chec

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added a comment. Just a couple tweaks and then LGTM. Comment at: lib/CodeGen/CGClass.cpp:1833 @@ +1832,3 @@ + // unless we are calling base constructor - we don't want to generating + // assumption loads for not completed because vptr may still change. + if (CGM.getC

Re: [PATCH] D12128: Generating available_externally vtables bugfix

rjmccall added a comment. LGTM. http://reviews.llvm.org/D12128 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12047: test/SemaObjC: Remove cruft in unused getter test

rjmccall added a comment. Sure, fine to me. http://reviews.llvm.org/D12047 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r245514 - Fix the layout of bitfields in ms_struct unions: their

Author: rjmccall Date: Wed Aug 19 17:42:36 2015 New Revision: 245514 URL: http://llvm.org/viewvc/llvm-project?rev=245514&view=rev Log: Fix the layout of bitfields in ms_struct unions: their alignment is ignored, and they always allocate a complete storage unit. Also, change the dumping of AST rec

r245771 - When building a pseudo-object assignment, and the RHS is

Author: rjmccall Date: Fri Aug 21 19:35:27 2015 New Revision: 245771 URL: http://llvm.org/viewvc/llvm-project?rev=245771&view=rev Log: When building a pseudo-object assignment, and the RHS is a contextually-typed expression that semantic analysis will probably need to invasively rewrite, don't inc

Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

rjmccall added a comment. This looks generally like what I'm looking for, thanks! Some comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1129 @@ +1128,3 @@ +if (GV && GV != GetGlobalValue(getMangledName(D))) + continue; + This is a pretty expensive e

Re: [PATCH] D12312: Emiting invariant.group.barrier and adding -fstrict-vptrs

rjmccall added a comment. You should add a test that actually checks that your feature works. Comment at: lib/CodeGen/CGClass.cpp:1279 @@ +1278,3 @@ + if (CGM.getCodeGenOpts().StrictVPtrs && BaseVPtrsInitialized) +CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXTh

Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

rjmccall added inline comments. Comment at: lib/CodeGen/CodeGenModule.h:354 @@ +353,3 @@ + /// call). + llvm::DenseSet ExplicitDefinitions; + andreybokhanko wrote: > Checking that a GlobalDecl is not in ExplicitDefinitions yet is actually > required to avoid pr

Re: [PATCH] D12389: Conditionalize X86 Darwin MaxVectorAlign on the presence of AVX.

rjmccall added a comment. Okay, and you're doing the avx512 part of this in a separate commit? LGTM. http://reviews.llvm.org/D12389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12390: Also enable the avx/avx512 ABIs for i386, not just x86_64.

rjmccall added a comment. This gives no-MMX priority over turning on SSE, which sounds like a major change in behavior to me. There are definitely clients that specifically never want to use MMX but do care deeply about SSE; my understanding is that modern microarchitectures heavily punish cas

Re: [PATCH] D12390: Also enable the avx/avx512 ABIs for i386, not just x86_64.

rjmccall added a comment. In http://reviews.llvm.org/D12390#234458, @ab wrote: > Unless I'm misunderstanding, I believe this has much less impact than you're > thinking; there are three cases: > > - x86_64: no change (-mno-mmx is guarded by x86) > - x86, with -mno-mmx: no change (because previou

Re: [PATCH] D12312: Emiting invariant.group.barrier and adding -fstrict-vptrs

rjmccall added inline comments. Comment at: include/clang/Driver/Options.td:990 @@ -988,2 +989,3 @@ "value range">; +def fstrict_vptrs: Flag<["-"], "fstrict-vptrs">, Group, Flags<[CC1Option]>; def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group; ---

Re: [PATCH] D12312: Emiting invariant.group.barrier and adding -fstrict-vptrs

rjmccall added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1279 @@ +1278,3 @@ + if (CGM.getCodeGenOpts().StrictVPtrs && BaseVPtrsInitialized) +CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXThis()); + Prazek wrote: > rjmccall wrote: > > Pr

Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

rjmccall added a comment. This looks great, thanks! One last comment; if you agree with me, go ahead and fix it and then commit. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2323 @@ -2323,1 +2322,3 @@ + "definition with same mangled name as another definition">, +

Re: [PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

rjmccall added a comment. Yes, please make it an error. And the obvious test changes are fine. :) http://reviews.llvm.org/D11297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r246986 - Move BlockByrefHelpers back to CodeGenModule.h to placate MSVC.

Author: rjmccall Date: Tue Sep 8 03:21:11 2015 New Revision: 246986 URL: http://llvm.org/viewvc/llvm-project?rev=246986&view=rev Log: Move BlockByrefHelpers back to CodeGenModule.h to placate MSVC. Modified: cfe/trunk/lib/CodeGen/CGBlocks.h cfe/trunk/lib/CodeGen/CodeGenModule.h Modified

r246988 - Remove unnecessary braces; this resolves against a

Author: rjmccall Date: Tue Sep 8 03:57:00 2015 New Revision: 246988 URL: http://llvm.org/viewvc/llvm-project?rev=246988&view=rev Log: Remove unnecessary braces; this resolves against a single-pointer overload instead of the ArrayRef one. Modified: cfe/trunk/lib/CodeGen/CGBuilder.h Modified:

Re: [PATCH] D13582: [DEBUG INFO] Emit debug info for type used in explicit cast only.

> On Oct 12, 2015, at 10:50 AM, David Blaikie wrote: > +John, author of the original patch - in case it's an obvious mistake. I > haven't looked at John's change yet & compared it to the intended behavior, > etc. Will do so soon and/or if John doesn't see something at a glance. The functionalit

Re: [PATCH] D13285: Fix for bug 24196: clang fails on assertion on complex doubles multiplication when EH is enabled

rjmccall added a comment. This is an inappropriate fix for this problem. If these runtime functions can never throw, which seems to be the case, you should create the function type with a no-throw exception specification, which will make EmitCall emit the call with a CallInst. EST_BasicNoexce

Re: [PATCH] D6700: Diagnose UnresolvedLookupExprs that resolve to instance members in static methods

rjmccall added inline comments. Comment at: lib/Sema/TreeTransform.h:9135 @@ +9134,3 @@ +if (NamedDecl *D = R.getAsSingle()) { + D = D->getUnderlyingDecl(); + if (isa(D) || isa(D) || getAsSingle already looks through to the underlying decl.

Re: [PATCH] D6700: Diagnose UnresolvedLookupExprs that resolve to instance members in static methods

rjmccall added a comment. As a more general comment, I believe the rule is that we try to always make a MemberExpr/UnresolvedMemberExpr whenever there *might* be a base, but that the resulting distinction between an implicit-base UnresolvedMemberExpr and an UnresolvedLookupExpr is not actually

Re: [PATCH] D13582: [DEBUG INFO] Emit debug info for type used in explicit cast only.

rjmccall added a comment. That looks great, thank you. http://reviews.llvm.org/D13582 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13336: [MSVC] 'property' with an empty array in array subscript expression.

rjmccall added a comment. I agree with Reid that you should not be adding a DenseMap to Sema for this. Just build a SubscriptExpr for the syntactic form and have it yield an expression of pseudo-object type; or you can make your own AST node for it if that makes things easier. http://reviews

Re: [PATCH] D13336: [MSVC] 'property' with an empty array in array subscript expression.

rjmccall added a comment. Needs more tests. 1. Dependent template instantiation. 2. Non-dependent template instantiation. 3. Indexes which are themselves pseudo-objects. 4. Templated getters and setters. 5. Non-POD by-value argument types. Comment at: include/clang/AST/BuiltinT

r250917 - Some minor ARC diagnostic improvements.

Author: rjmccall Date: Wed Oct 21 13:06:38 2015 New Revision: 250917 URL: http://llvm.org/viewvc/llvm-project?rev=250917&view=rev Log: Some minor ARC diagnostic improvements. Modified: cfe/trunk/include/clang/AST/Type.h cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/li

r250918 - Unify the ObjC entrypoint caches.

Author: rjmccall Date: Wed Oct 21 13:06:43 2015 New Revision: 250918 URL: http://llvm.org/viewvc/llvm-project?rev=250918&view=rev Log: Unify the ObjC entrypoint caches. Modified: cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/CodeGen/CGObjC.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h

r250919 - Fix and stylize the emission of GC/ARC ivar and GC block layout strings.

Author: rjmccall Date: Wed Oct 21 13:06:47 2015 New Revision: 250919 URL: http://llvm.org/viewvc/llvm-project?rev=250919&view=rev Log: Fix and stylize the emission of GC/ARC ivar and GC block layout strings. Specifically, handle under-aligned object references (by explicitly ignoring them, becaus

r250916 - In ARC, peephole the initialization of a __weak variable with

Author: rjmccall Date: Wed Oct 21 13:06:31 2015 New Revision: 250916 URL: http://llvm.org/viewvc/llvm-project?rev=250916&view=rev Log: In ARC, peephole the initialization of a __weak variable with a value loaded from a __weak variable into a call to objc_copyWeak or objc_moveWeak. Modified: c

Re: [PATCH] D12614: [OpenMP] Offloading descriptor registration and device codegen.

rjmccall added a comment. CurFuncDecl is supposed to be the enclosing user function. Things like outlined functions should be getting stored in CurCodeDecl; that's how it's done for blocks and lambdas. http://reviews.llvm.org/D12614 ___ cfe-commi

Re: [PATCH] D13954: CodeGen: Fix LLVM assertion if Swift and Clang emit Objective-C class reference in same LLVM module

rjmccall added a comment. It just occurred to me that there is a way to test this in Clang with the asm-label extension: int Foo_class asm("OBJC_CLASS_$_Foo"); Of course, you'll have to actually use it from somewhere, or define it, in order for it to actually show up in the IR and cause a co

r250955 - Enable ARC on the fragile runtime.

Author: rjmccall Date: Wed Oct 21 17:06:03 2015 New Revision: 250955 URL: http://llvm.org/viewvc/llvm-project?rev=250955&view=rev Log: Enable ARC on the fragile runtime. This is almost entirely a matter of just flipping a switch. 99% of the runtime support is available all the way back to when i

Re: [PATCH] D13959: Fix crash in EmitDeclMetadata mode

rjmccall added a comment. LGTM. Repository: rL LLVM http://reviews.llvm.org/D13959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r251041 - Define weak and __weak to mean ARC-style weak references, even in MRC.

Author: rjmccall Date: Thu Oct 22 13:38:17 2015 New Revision: 251041 URL: http://llvm.org/viewvc/llvm-project?rev=251041&view=rev Log: Define weak and __weak to mean ARC-style weak references, even in MRC. Previously, __weak was silently accepted and ignored in MRC mode. That makes this a potenti

Re: [PATCH] D13336: [MSVC] 'property' with an empty array in array subscript expression.

rjmccall added inline comments. Comment at: include/clang/AST/ExprCXX.h:689 @@ +688,3 @@ +/// indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b), and +/// p->x[a][b] = i will be turned into p->PutX(a, b, i). +class MSPropertySubscriptExpr : public Expr {

r251384 - Be more conservative about diagnosing "incorrect" uses of __weak:

Author: rjmccall Date: Mon Oct 26 23:54:50 2015 New Revision: 251384 URL: http://llvm.org/viewvc/llvm-project?rev=251384&view=rev Log: Be more conservative about diagnosing "incorrect" uses of __weak: allow them to be written in certain kinds of user declaration and diagnose on the use-site instea

r251469 - Add the ability to define "fake" arguments on attributes.

Author: rjmccall Date: Tue Oct 27 19:17:34 2015 New Revision: 251469 URL: http://llvm.org/viewvc/llvm-project?rev=251469&view=rev Log: Add the ability to define "fake" arguments on attributes. Fake arguments are automatically handled for serialization, cloning, and other representational tasks, b

r251496 - Refine r251469 to give better (and more localizable) diagnostics

Author: rjmccall Date: Wed Oct 28 00:03:19 2015 New Revision: 251496 URL: http://llvm.org/viewvc/llvm-project?rev=251496&view=rev Log: Refine r251469 to give better (and more localizable) diagnostics for all the reasons that ARC makes things implicitly unavailable. Modified: cfe/trunk/include

Re: [PATCH] D14149: __builtin_signbit fix for ppcf128 type

rjmccall added a comment. Just a few comment suggestions, but functionally LGTM. Comment at: lib/CodeGen/CGBuiltin.cpp:246 @@ -244,1 +245,3 @@ +// On little-Endian, high-double will be in low part of i128. +// Therefore, on big-Endian we shift high part to low part.

Re: [PATCH] D12793: Three new overflow builtins with generic argument types

rjmccall added a comment. Sorry for the delay. Committed with a few minor tweaks in r251650. http://reviews.llvm.org/D12793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r251651 - Add support for __builtin_{add,sub,mul}_overflow.

Author: rjmccall Date: Thu Oct 29 15:48:01 2015 New Revision: 251651 URL: http://llvm.org/viewvc/llvm-project?rev=251651&view=rev Log: Add support for __builtin_{add,sub,mul}_overflow. Patch by David Grayson! Added: cfe/trunk/test/Sema/builtins-overflow.c Modified: cfe/trunk/docs/Languag

r251666 - Fix the emission of ARC ivar layouts in the non-fragile Mac runtime.

Author: rjmccall Date: Thu Oct 29 18:36:14 2015 New Revision: 251666 URL: http://llvm.org/viewvc/llvm-project?rev=251666&view=rev Log: Fix the emission of ARC ivar layouts in the non-fragile Mac runtime. My previous change in this area accidentally broke the rule when InstanceBegin was not a mult

Re: [PATCH] D12614: [OpenMP] Offloading descriptor registration and device codegen.

rjmccall added a comment. In http://reviews.llvm.org/D12614#274349, @sfantao wrote: > Hi John, > > Thanks for the remark! > > In http://reviews.llvm.org/D12614#272354, @rjmccall wrote: > > > CurFuncDecl is supposed to be the enclosing user function. Things like > > outlined functions should be

r251677 - Initialize @catch variables correctly in fragile-runtime ARC.

Author: rjmccall Date: Thu Oct 29 19:56:02 2015 New Revision: 251677 URL: http://llvm.org/viewvc/llvm-project?rev=251677&view=rev Log: Initialize @catch variables correctly in fragile-runtime ARC. Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp cfe/trunk/lib/CodeGen/CGObjCRuntime.cpp cf

Re: r251041 - Define weak and __weak to mean ARC-style weak references, even in MRC.

> On Oct 30, 2015, at 5:39 PM, Nico Weber wrote: > Hi John, > > this breaks programs that use __weak and target 10.6, like so: > > $ cat test.m > #import > @interface I : NSObject { > __weak NSObject* foo_; > } > - (NSObject*)foo; > @end > > @implementation I > - (NSObject *)foo { > return

Re: [PATCH] D14149: __builtin_signbit fix for ppcf128 type

rjmccall added a comment. LGTM, thanks! Repository: rL LLVM http://reviews.llvm.org/D14149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r251041 - Define weak and __weak to mean ARC-style weak references, even in MRC.

> On Nov 2, 2015, at 2:53 PM, Nico Weber wrote: > On Fri, Oct 30, 2015 at 5:55 PM, John McCall > wrote: > > On Oct 30, 2015, at 5:39 PM, Nico Weber > > wrote: > > Hi John, > > > > this breaks programs that use __weak and target 10.6, like so

r252187 - After some discussion, promote -fobjc-weak to a driver option.

Author: rjmccall Date: Thu Nov 5 13:19:56 2015 New Revision: 252187 URL: http://llvm.org/viewvc/llvm-project?rev=252187&view=rev Log: After some discussion, promote -fobjc-weak to a driver option. rdar://problem/23415863 Added: cfe/trunk/test/Driver/objc-weak.m Modified: cfe/trunk/inclu

Re: [PATCH] D12614: [OpenMP] Offloading descriptor registration and device codegen.

rjmccall added a comment. In http://reviews.llvm.org/D12614#284158, @sfantao wrote: > As for the structor variants, I am now using the complete variant to generate > the names of the kernels as you suggested. I didn't add any method to CXXABI > as that will require extra logic in ASTContext to

Re: [Diffusion] rL246882: Don't crash on a self-alias declaration

rjmccall added a subscriber: rjmccall. rjmccall added a comment. Hmm. That cast to GlobalAlias seems quite brittle anyway; it would be good to fix it (and diagnose the fact that forming the alias failed). But this change seems independently useful. Approved for 3.7. Users: hfinkel (Author

r252668 - Define __unsafe_unretained and __autoreleasing in ObjC GC mode.

Author: rjmccall Date: Tue Nov 10 17:00:25 2015 New Revision: 252668 URL: http://llvm.org/viewvc/llvm-project?rev=252668&view=rev Log: Define __unsafe_unretained and __autoreleasing in ObjC GC mode. This was an accidental regression from the MRC __weak patch. Modified: cfe/trunk/lib/Frontend

r276180 - When copying an array into a lambda, destroy temporaries from

Author: rjmccall Date: Wed Jul 20 16:02:43 2016 New Revision: 276180 URL: http://llvm.org/viewvc/llvm-project?rev=276180&view=rev Log: When copying an array into a lambda, destroy temporaries from the copy-constructor immediately and enter a partial array cleanup for previously-copied elements. F

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

rjmccall added a comment. In https://reviews.llvm.org/D22666#493026, @hfinkel wrote: > What's the actual desired behavior here? Should the inliner strip out calls > to mcount() as it inlines? I think they basically just want a late pass (as in immediately prior to codegen) to insert the mcoun

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

rjmccall added a comment. Note that the presence of the mcount call itself will significantly impact inlining and, really, the entire optimization pipeline. Having this be a late pass seems more in keeping with what I assume is a goal that this instrumentation doesn't drastically change the ge

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

rjmccall added a comment. LGTM, too. https://reviews.llvm.org/D22666 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22666: Frontend: Fix mcount inlining bug

rjmccall added a comment. In https://reviews.llvm.org/D22666#500326, @honggyu.kim wrote: > Should we also modify clang/test/CodeGen/mcount.c as well? I'm not actually > familiar with test infra. Yes, you'll need to modify it to test for the attribute instead. https://reviews.llvm.org/D22666

Re: [PATCH] D16788: PS4 ABI Round 2. Actual PS4 code.

rjmccall added inline comments. Comment at: include/clang/Basic/TargetCXXABI.h:118 @@ -115,1 +117,3 @@ +/// in LLVM 3.2. +PS4 }; I'm not sure why you added a new C++ ABI kind here. The bug fix you're opting out of is not at all specific to C++, and th

Re: [PATCH] D16788: PS4 ABI Round 2. Actual PS4 code.

rjmccall added inline comments. Comment at: include/clang/Basic/TargetCXXABI.h:118 @@ -115,1 +117,3 @@ +/// in LLVM 3.2. +PS4 }; probinson wrote: > rjmccall wrote: > > I'm not sure why you added a new C++ ABI kind here. The bug fix you're > > opting o

Re: [PATCH] D16819: Remove llvm::(cast|isa) from lib/CodeGen/Address.h to fix build with gcc-4.8.1

rjmccall added a comment. LGTM, too. http://reviews.llvm.org/D16819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16846: PR26449: Fixes for bugs in __builtin_classify_type implementation

rjmccall added inline comments. Comment at: lib/AST/ExprConstant.cpp:6213 @@ -6211,3 +6212,3 @@ else if (ArgTy->isEnumeralType()) -return enumeral_type_class; +return LangOpts.CPlusPlus ? enumeral_type_class : integer_type_class; else if (ArgTy->isBooleanType())

Re: [PATCH] D16846: PR26449: Fixes for bugs in __builtin_classify_type implementation

rjmccall added a comment. Thank you, that's a lot better. Just a few suggestions to promote exhaustive checking. Comment at: lib/AST/ExprConstant.cpp:6246 @@ +6245,3 @@ + +default: + break; Again, try to avoid adding default cases. There's an x-macro

Re: [PATCH] D16788: PS4 ABI Round 2. Actual PS4 code.

rjmccall added a comment. Yes, thank you, this looks great. I agree with David's review, but otherwise this LGTM. http://reviews.llvm.org/D16788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

r259932 - Add an ARC autoreleased-return-value caller marker on i386.

Author: rjmccall Date: Fri Feb 5 15:37:38 2016 New Revision: 259932 URL: http://llvm.org/viewvc/llvm-project?rev=259932&view=rev Log: Add an ARC autoreleased-return-value caller marker on i386. rdar://24531556 Added: cfe/trunk/test/CodeGenObjC/arc-i386.m Modified: cfe/trunk/lib/CodeGen/

Re: [PATCH] D16846: PR26449: Fixes for bugs in __builtin_classify_type implementation

rjmccall added a comment. Yes, thank you, that looks great. LGTM. http://reviews.llvm.org/D16846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

rjmccall added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7829 @@ -7828,2 +7828,3 @@ UnresolvedSet<8> Matches; + FunctionDecl *Specialization = nullptr; TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc()); Please name this variable N

Re: [PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

rjmccall added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7845 @@ -7842,1 +7844,3 @@ + } else +NonTemplateMatch = Method; } Could you add an assertion here that NonTemplateMatch is still null? That should definitely neve

Re: [PATCH] D17215: [Sema] Fix PR14211 Crash for explicit instantiation of overloaded template function within class template

rjmccall added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7845 @@ -7842,1 +7844,3 @@ + } else +NonTemplateMatch = Method; } hintonda wrote: > rjmccall wrote: > > Could you add an assertion here that NonTemplateMatch is sti

Re: [PATCH] D17023: pr26544: Bitfield layout with pragma pack and attributes "packed" and "aligned

rjmccall added a comment. LGTM. http://reviews.llvm.org/D17023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16460: Bug 10002 - [opencl] Wrongfully assuming RHS is always unsigned

rjmccall added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:2716 @@ +2715,3 @@ + if (Ops.LHS->getType() != RHS->getType()) { +bool isSigned = dyn_cast(Ops.E)->getRHS()->getType().getTypePtr()->isSignedIntegerType(); +RHS = Builder.CreateIntCast(RHS, Ops.LHS-

Re: [PATCH] D19589: [clang] [Builtin] Make __builtin_thread_pointer target-independent.

rjmccall added a comment. Why the random triple change on the test? Otherwise LGTM. Repository: rL LLVM http://reviews.llvm.org/D19589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

Re: [PATCH] D19589: [clang] [Builtin] Make __builtin_thread_pointer target-independent.

rjmccall added a comment. Oh, I see. Sure, LGTM. Repository: rL LLVM http://reviews.llvm.org/D19589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19432: [SystemZ] Support Swift calling convention

rjmccall added a comment. LGTM. http://reviews.llvm.org/D19432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

rjmccall added a comment. I see what you're going for with "listing file", but I like ICC's option name much better, or at least something along those lines. http://reviews.llvm.org/D19678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

rjmccall added a comment. In http://reviews.llvm.org/D19678#416059, @hfinkel wrote: > In http://reviews.llvm.org/D19678#415844, @rjmccall wrote: > > > I see what you're going for with "listing file", but I like ICC's option > > name much better, or at least something along those lines. > > > Sou

Re: [PATCH] D19536: [CodeGenObjCXX] Fix handling of blocks in lambda

rjmccall added inline comments. Comment at: lib/CodeGen/CGBlocks.cpp:806 @@ -792,1 +805,3 @@ + } + src = Builder.CreateStructGEP(Addr, Idx, Offset, FD->getName()); } else { Hmm. It's become increasingly clear that my original decision to expand the

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

rjmccall added a comment. This discussion of the command line interface makes me think that we should be taking Richard's suggestion one step further. Why is Clang's involvement here more than just handing down specific requests for optimization data to LLVM and packaging that information back

Re: [PATCH] D19678: Annotated-source optimization reports (a.k.a. "listing" files)

rjmccall added a comment. In http://reviews.llvm.org/D19678#420358, @hfinkel wrote: > In http://reviews.llvm.org/D19678#419445, @rjmccall wrote: > > > This discussion of the command line interface makes me think that we should > > be taking Richard's suggestion one step further. Why is Clang's

Re: [PATCH] D19536: [CodeGenObjCXX] Fix handling of blocks in lambda

rjmccall added a comment. Yes, that looks good, thanks. http://reviews.llvm.org/D19536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18815: [ObjC] Enter a new evaluation context before calling BuildBlockForLambdaConversion.

rjmccall added a comment. LGTM. http://reviews.llvm.org/D18815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20113: Fix mangled name of method with ns_consumed parameters.

rjmccall added a comment. This is a good catch, thanks! As a slight adjustment, It's probably better to just ignore this attribute when mangling the function type of an entity, the same way that we generally don't mangle return types because they don't affect overloading. That will require an

Re: [PATCH] D20113: Fix mangled name of method with ns_consumed parameters.

rjmccall added a comment. In http://reviews.llvm.org/D20113#426884, @sdefresne wrote: > In http://reviews.llvm.org/D20113#425984, @rjmccall wrote: > > > This is a good catch, thanks! > > > Thank you for the quick reply. > > Please excuse me if I misunderstood you or if my remark appear off the ma

Re: [PATCH] D20045: [ObjC][CodeGen] Remove an assert that is no longer correct.

rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:365 @@ +364,3 @@ + // If the global variable already has an initializer, there is no need to + // emit code for initialization or push a cleanup. + if (Var->hasInitializer()) As we d

Re: [PATCH] D20045: [ObjC][CodeGen] Remove an assert that is no longer correct.

rjmccall added a comment. LGTM, thanks. http://reviews.llvm.org/D20045 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r269880 - Various improvements to the public IRGen interface.

Author: rjmccall Date: Wed May 18 00:21:18 2016 New Revision: 269880 URL: http://llvm.org/viewvc/llvm-project?rev=269880&view=rev Log: Various improvements to the public IRGen interface. Modified: cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h cfe/trunk/include/clang/CodeGen/ModuleBuil

Re: [PATCH] D20407: [CodeGen][ObjC] zero-ext an i1 value to i8

rjmccall added a comment. _Atomic is functionally a type qualifier and should be removed in Sema when computing the result type of the getter and the parameter type of the setter. That is, if the user declares a property of type _Atomic(_Bool), we should pretend that the property has type _Boo

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