Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

rjmccall added a comment. Thanks! Comment at: lib/Sema/SemaPseudoObject.cpp:249 @@ -248,1 +248,3 @@ +virtual bool useSetterResultAsExprResult(Expr *) const { return false; } +virtual bool captureSetValueAsResult() const { return true; } }; I think

Re: [PATCH] D15120: Add support for __float128 type to be used by targets that support it

rjmccall added a reviewer: akyrtzi. Comment at: lib/AST/ASTContext.cpp:5424 @@ -5410,1 +5423,3 @@ +// FIXME: need to figure out what this is for __float128 +case BuiltinType::Float128: return 'K'; case BuiltinType::NullPtr:return '*'; // like char*

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

rjmccall added a comment. I don't understand why that's true. buildSet is called with captureSetValueAsResult=true, and the set value is definitely capturable, so that value should be set as the result; and you're not overriding it. Why does the expression end up having type void?

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

rjmccall added a comment. Oh, I see, of course. It would be clearer if you asked the subclass which value to use abstractly (i.e. independently of the actual set expression) and then passed down captureSetValueAsResult=false when the subclass says to use the setter result.

Re: [PATCH] Add ObjCBoxable handling to ObjCMigrator

> On Dec 5, 2015, at 7:58 AM, AlexDenisov <1101.deb...@gmail.com> wrote: > Extend ObjCMigrator to cover automatic migration from message sending to > boxable literals, e.g.: > > ```before > typedef struct __attribute__((objc_boxable)) CGRect CGRect; > /// ... > CGRect rect; > [NSValue

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

rjmccall added a comment. In http://reviews.llvm.org/D15174#302248, @ABataev wrote: > John, > the result is always the result of Put operation. For pre-increment we > have to return new value, for the post-increment - previous value > returned by Get (checked it on MSVC). > So, currently

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

rjmccall added inline comments. Comment at: lib/AST/RecordLayoutBuilder.cpp:1606 @@ +1605,3 @@ +} else if (ExplicitFieldAlign && + Context.getTargetInfo().useBitFieldTypeAlignment()) + FieldOffset = llvm::RoundUpToAlignment(FieldOffset, ExplicitFieldAlign);

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

rjmccall added inline comments. Comment at: lib/Sema/SemaPseudoObject.cpp:232 @@ -231,3 +231,3 @@ /// Return true if assignments have a non-void result. -bool CanCaptureValue(Expr *exp) { +bool CanCaptureValue(Expr *exp) const { if (exp->isGLValue())

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

rjmccall added a comment. Yes, I think that's a reasonable approach. http://reviews.llvm.org/D14980 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15174: [MSVC] Fix for http://llvm.org/PR25636: indexed accessor property not supported correctly.

rjmccall added a comment. Hmm. I think a better approach would be for buildAssignmentOperation to do this; but before we figure out how to do that, we should make sure of the language semantics we're implementing. Are the semantics that the result of an assignment is always the result of the

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

rjmccall added inline comments. Comment at: lib/AST/RecordLayoutBuilder.cpp:1606 @@ -1605,1 +1605,3 @@ +} else if (ExplicitFieldAlign) + FieldOffset = llvm::RoundUpToAlignment(FieldOffset, ExplicitFieldAlign); Be sure to test specifically with an APCS

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

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

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

rjmccall added a comment. I don't mean the actual layout used by Windows targets; I mean the layout used by the ms_struct pragma / attribute. Comment at: lib/AST/RecordLayoutBuilder.cpp:1606 @@ -1605,1 +1605,3 @@ +} else if (ExplicitFieldAlign) + FieldOffset =

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

rjmccall added a comment. Please feel free to commit after you make this last change. Comment at: lib/CodeGen/TargetInfo.cpp:216 @@ +215,3 @@ + } + else { +Addr = Address(Ptr, SlotSize); LLVM code style is to put the else on the same line as the closing

Re: [PATCH] D14872: PR25575: Make GCC 4.4+ comatible layout for packed bit-fileds of char type

rjmccall added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2790 @@ -2790,1 +2789,3 @@ + "version 4.4 - the newer offset is used here">, + InGroup>; def

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

rjmccall added a comment. Thank you. A few more style complaints; with those fixed, LGTM. Comment at: lib/CodeGen/TargetInfo.cpp:166 @@ +165,3 @@ +// Dynamically round a pointer up to a multiple of the given alignment. +static llvm::Value*

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

rjmccall added a comment. Well, this is a really nasty bug. Please check how this interacts with #pragma pack, which normally takes precedence over even explicit alignment attributes. If that's the case here, then what you really want to do is just add the same "ExplicitFieldAlign ||" clause

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

rjmccall added a comment. In http://reviews.llvm.org/D14980#298754, @rsmith wrote: > GCC's behavior (`aligned` on a field specifies the alignment of the start of > that field) makes a little more sense to me than Clang's behavior (the type > and alignment of a field specify a flavour of

Re: [PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

On Mon, Nov 30, 2015 at 2:31 PM, Richard Smith wrote: > On Mon, Nov 30, 2015 at 11:33 AM, John McCall wrote: > >> rjmccall added a comment. >> >> In http://reviews.llvm.org/D14980#298754, @rsmith wrote: >> >> > GCC's behavior (`aligned` on a field

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:241 @@ +240,3 @@ + return Address(PtrAsInt, Align); +} + Thank you for extracting this. First, this function deserves a doc comment now; I would suggest: /// Dynamically round a pointer

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

rjmccall added a comment. This looks fantastic, thanks for doing all that. Approved with the one minor change. Comment at: lib/Sema/SemaPseudoObject.cpp:175 @@ -145,1 +174,3 @@ +PSE->getType(), PSE->getValueKind(), PSE->getObjectKind(), +

Re: [PATCH] D14864: [X86] Support for C calling convention only for MCU target.

rjmccall added a comment. LGTM. http://reviews.llvm.org/D14864 ___ 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 inline comments. Comment at: lib/Sema/SemaPseudoObject.cpp:57 @@ +56,3 @@ + // with a base limits the number of cases here. + assert(refExpr->isObjectReceiver()); + Well, it should be okay to call this on something that doesn't have a

Re: [PATCH] D14871: [Power PC] fix calculating address of arguments on stack for variadic functions

rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:3548 @@ +3547,3 @@ +llvm::Value *OverflowArgArea = OverflowArea.getPointer(); +uint32_t Align = CGF.getContext().getTypeAlignInChars(Ty).getQuantity(); +if (Align > 4) { This patch

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

rjmccall added a comment. In http://reviews.llvm.org/D14737#293967, @pete wrote: > Added a couple of tests for retain returning types other than id. Returning > a pointer should still be converted to a call, while returning a non-pointer > such as float will get a message instead. > > I

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

rjmccall added a comment. The casts done by emitARCValueOperation will handle the input, but they don't quite handle the result properly. The right test case here is a method named "retain" that's declared to return something completely unrelated to its receiver type, e.g. @class A;

Re: [PATCH] D14796: Preserve exceptions information during calls code generation.

rjmccall added a comment. What I was thinking was something more along the lines of a little struct that stored either a Decl * or a FunctionType *, and you could change the TargetDecl argument to functions like EmitCall and ConstructAttributeList to that. Maybe call it something like

r253534 - Don't actually add the __unsafe_unretained qualifier in MRC;

Author: rjmccall Date: Wed Nov 18 20:28:03 2015 New Revision: 253534 URL: http://llvm.org/viewvc/llvm-project?rev=253534=rev Log: Don't actually add the __unsafe_unretained qualifier in MRC; driving a canonical difference between that and an unqualified type is a really bad idea when both are

r253533 - Fix the emission of ARC-style ivar layouts in the fragile runtime

Author: rjmccall Date: Wed Nov 18 20:27:55 2015 New Revision: 253533 URL: http://llvm.org/viewvc/llvm-project?rev=253533=rev Log: Fix the emission of ARC-style ivar layouts in the fragile runtime to start at the offset of the first ivar instead of the rounded-up end of the superclass. The latter

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

rjmccall added inline comments. Comment at: include/clang/Basic/ObjCRuntime.h:182 @@ +181,3 @@ +switch (getKind()) { +case FragileMacOSX: return false; +case MacOSX: return getVersion() >= VersionTuple(10, 10); I went ahead and asked Greg, and he

r253255 - Correctly handle type mismatches in the __weak copy/move-initialization

Author: rjmccall Date: Mon Nov 16 16:11:41 2015 New Revision: 253255 URL: http://llvm.org/viewvc/llvm-project?rev=253255=rev Log: Correctly handle type mismatches in the __weak copy/move-initialization peephole I added in r250916. rdar://23559789 Added: cfe/trunk/test/CodeGenObjC/arc-weak.m

r252971 - Remove -Wobjc-weak-compat; there isn't a compelling use case for this.

Author: rjmccall Date: Thu Nov 12 17:39:39 2015 New Revision: 252971 URL: http://llvm.org/viewvc/llvm-project?rev=252971=rev Log: Remove -Wobjc-weak-compat; there isn't a compelling use case for this. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

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

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

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=rev Log: After some discussion, promote -fobjc-weak to a driver option. rdar://problem/23415863 Added: cfe/trunk/test/Driver/objc-weak.m Modified:

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, >

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 > -

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: [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=rev Log: Add support for __builtin_{add,sub,mul}_overflow. Patch by David Grayson! Added: cfe/trunk/test/Sema/builtins-overflow.c Modified:

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=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 multiple

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

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.

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=rev Log: Add the ability to define "fake" arguments on attributes. Fake arguments are automatically handled for serialization, cloning, and other representational tasks, but

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=rev Log: Refine r251469 to give better (and more localizable) diagnostics for all the reasons that ARC makes things implicitly unavailable. Modified:

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=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 instead.

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=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 potentially

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

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=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=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, because

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=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:

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 ___

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

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=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 it

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:

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.

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] 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] 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.

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

Re: [PATCH] D13325: Fix crash in codegen on casting to `bool &`.

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

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=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:

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=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

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

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: [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

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=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 or

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. Thanks for doing this; this is a great start. Comment at: docs/LanguageExtensions.rst:1720 @@ -1712,1 +1719,3 @@ +being stored there, and the function returns 1. The behavior of these builtins +is well-defined for all argument values.

r247597 - Fix a nasty bug with the partial destruction of nested arrays;

Author: rjmccall Date: Mon Sep 14 13:57:08 2015 New Revision: 247597 URL: http://llvm.org/viewvc/llvm-project?rev=247597=rev Log: Fix a nasty bug with the partial destruction of nested arrays; it escaped notice because it's only used for heterogeneous initialization. rdar://21397946 Modified:

Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst

rjmccall added a comment. In http://reviews.llvm.org/D12743#244375, @vsk wrote: > Ah, ok. We have some objective-c++ code which calls into a boost routine > which throws an exception. That results in an undefined reference to > ___objc_personality_v0, because the boost library we linked

r247482 - When comparing two block captures for layout, don't crash

Author: rjmccall Date: Fri Sep 11 17:00:51 2015 New Revision: 247482 URL: http://llvm.org/viewvc/llvm-project?rev=247482=rev Log: When comparing two block captures for layout, don't crash if they have the same alignment and one was 'this'. Fixes PR24780. Modified:

Re: [PATCH] D11361: [OpenMP] Target directive host codegen

rjmccall added a comment. Sorry for putting off the final review on this; I was heads-down trying to get the alignment patch done. It's looking good; obviously you'll need to update it to work with Addresses properly, but hopefully that won't be too much of a problem. When you do, maybe you

Re: [PATCH] D12743: [CodeGen] Teach SimplifyPersonality about the updated LandingPadInst

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

r247207 - Fix access control for lookups using the Microsoft __super extension.

Author: rjmccall Date: Wed Sep 9 18:04:17 2015 New Revision: 247207 URL: http://llvm.org/viewvc/llvm-project?rev=247207=rev Log: Fix access control for lookups using the Microsoft __super extension. rdar://22464808 Added: cfe/trunk/test/SemaCXX/microsoft-super.cpp Modified:

r247209 - ARC: Fix the precise-lifetime suppression of returns_inner_pointer

Author: rjmccall Date: Wed Sep 9 18:37:17 2015 New Revision: 247209 URL: http://llvm.org/viewvc/llvm-project?rev=247209=rev Log: ARC: Fix the precise-lifetime suppression of returns_inner_pointer receiver extension for message sends via property syntax. rdar://22172983 Modified:

r247228 - Don't crash when emitting a block under returns_nonnull.

Author: rjmccall Date: Wed Sep 9 19:57:46 2015 New Revision: 247228 URL: http://llvm.org/viewvc/llvm-project?rev=247228=rev Log: Don't crash when emitting a block under returns_nonnull. rdar://22071955 Added: cfe/trunk/test/CodeGen/sanitize-blocks.c Modified:

r246991 - When building the alloca for a local variable, set its name

Author: rjmccall Date: Tue Sep 8 04:18:30 2015 New Revision: 246991 URL: http://llvm.org/viewvc/llvm-project?rev=246991=rev Log: When building the alloca for a local variable, set its name separately from building the instruction so that it's preserved even in -Asserts builds. Employ C++'s

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=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: r246991 - When building the alloca for a local variable, set its name

> On Sep 8, 2015, at 10:52 AM, Chandler Carruth wrote: > None of my performance concerns were relevant to this change FWIW. > > I think the reason that this got "fixed" was because people had a tendancy to > *rely* on the name downstream when we made it always present. =/

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

> On Sep 8, 2015, at 6:19 PM, Steven Wu wrote: > CreateElementBitcast doesn’t seem to do what the name suggested. If you give > it VTy, it doesn’t grab the element type to generate bitcast. Is this > by-design? If so, I need to do this: > PtrOp0 =

Re: r246991 - When building the alloca for a local variable, set its name

> On Sep 8, 2015, at 8:25 AM, David Blaikie <dblai...@gmail.com> wrote: > On Tue, Sep 8, 2015 at 2:18 AM, John McCall via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > Author: rjmccall > Date: Tue Sep 8 04:18:30 2015 >

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

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

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::DenseSetGlobalDecl ExplicitDefinitions; + andreybokhanko wrote: Checking that a GlobalDecl is not in ExplicitDefinitions yet is actually required to

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 previously,

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, Groupf_Group, Flags[CC1Option]; def fstrict_overflow : Flag[-], fstrict-overflow, Groupf_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: Prazek

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 =

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

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=245771view=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

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

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=245514view=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

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

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

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 for

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 +

<    2   3   4   5   6   7   8   >