Re: r262278 - Generalize the consumed-parameter array on FunctionProtoType

> On Feb 29, 2016, at 5:06 PM, Pariborz Jahanian wrote: >> On Feb 29, 2016, at 4:49 PM, John McCall via cfe-commits >> wrote: >> >> Author: rjmccall >> Date: Mon Feb 29 18:49:02 2016 >> New Revision: 262278 >> >> URL: http://llvm

r262278 - Generalize the consumed-parameter array on FunctionProtoType

Author: rjmccall Date: Mon Feb 29 18:49:02 2016 New Revision: 262278 URL: http://llvm.org/viewvc/llvm-project?rev=262278&view=rev Log: Generalize the consumed-parameter array on FunctionProtoType to allow arbitrary data to be associated with a parameter. Also, fix a bug where we apparently haven'

r262275 - Infrastructure improvements to Clang attribute TableGen.

Author: rjmccall Date: Mon Feb 29 18:18:05 2016 New Revision: 262275 URL: http://llvm.org/viewvc/llvm-project?rev=262275&view=rev Log: Infrastructure improvements to Clang attribute TableGen. This should make it easier to add new Attr subclasses. Modified: cfe/trunk/include/clang/AST/Attr.h

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] 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] 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] 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:7829 @@ -7828,2 +7828,3 @@ UnresolvedSet<8> Matches; + FunctionDecl *Specialization = nullptr; TemplateSpecCandidateSet FailedCandidates(D.getIdentifierLoc()); Please name this variable N

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

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

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] 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] 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] 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] 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] D15120: Add support for __float128 type to be used by targets that support it

rjmccall added a comment. In http://reviews.llvm.org/D15120#337975, @hubert.reinterpretcast wrote: > In http://reviews.llvm.org/D15120#337654, @rjmccall wrote: > > > In http://reviews.llvm.org/D15120#337552, @hubert.reinterpretcast wrote: > > > > > It remains that the present standardization effo

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

rjmccall added a comment. In http://reviews.llvm.org/D15120#337552, @hubert.reinterpretcast wrote: > In http://reviews.llvm.org/D15120#337384, @rjmccall wrote: > > > I think it's not unlikely that float128_t will see future standardization > > (as an optional extension, of course), and it would

r258962 - Emit calls to objc_unsafeClaimAutoreleasedReturnValue when

Author: rjmccall Date: Wed Jan 27 12:32:30 2016 New Revision: 258962 URL: http://llvm.org/viewvc/llvm-project?rev=258962&view=rev Log: Emit calls to objc_unsafeClaimAutoreleasedReturnValue when reclaiming a call result in order to ignore it or assign it to an __unsafe_unretained variable. This av

Re: [PATCH] D16607: Implementation of PS4 ABI, round 1

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

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

rjmccall added a comment. I think it's not unlikely that float128_t will see future standardization (as an optional extension, of course), and it would be strange for an operation between two types to not consistently yield the type of higher rank. I could see an argument that we should not tre

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

rjmccall added a comment. In http://reviews.llvm.org/D15120#336996, @hubert.reinterpretcast wrote: > In http://reviews.llvm.org/D15120#336891, @rjmccall wrote: > > > > The C committee decided that "undefined behavior" was what they could > > > agree on for this sort of case. > > > > > > That's o

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

rjmccall added a comment. In http://reviews.llvm.org/D15120#336855, @hubert.reinterpretcast wrote: > In http://reviews.llvm.org/D15120#336827, @rjmccall wrote: > > > Here's the thing, though: I don't think there's a reasonable language > > solution here besides saying that float128_t has higher

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

rjmccall added a comment. In http://reviews.llvm.org/D15120#336776, @hubert.reinterpretcast wrote: > In http://reviews.llvm.org/D15120#336430, @rjmccall wrote: > > > As I understand it, PPC's long-double (~103 bits of precision) is still > > strictly less precise than float128_t (113 bits of pre

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

rjmccall added a comment. In http://reviews.llvm.org/D15120#336282, @nemanjai wrote: > Addressed review comments. > The key differences are: > > - No assignments or operations between entities of long double and __float128 > allowed if the two types have a different representation > - Each type

Re: [PATCH] D16586: Make clang AAPCS compliant w.r.t volatile bitfield accesses

rjmccall added a comment. Well, that's certainly an interesting ABI rule. A few high-level notes: 1. AAPCS requires the bit-field to be loaded on a store, even if the store fills the entire container; that doesn't seem to be implemented in your patch. 2. Especially because of #1, let's not do

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

rjmccall added inline comments. Comment at: include/clang/Basic/TargetInfo.h:385 @@ +384,3 @@ + unsigned getFloat128Width() const { return Float128Width; } + unsigned getFloat128Align() const { return Float128Align; } + const llvm::fltSemantics &getFloat128Format() const {

Re: [PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

rjmccall added a comment. Yes, this seems to be exactly what I wanted, thanks! LGTM. http://reviews.llvm.org/D15686 ___ 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. Sorry, holidays. I'm comfortable with taking this patch as-is. 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: r256996 - Properly bind up any cleanups in an ExprWithCleanups after

> On Jan 6, 2016, at 3:54 PM, Richard Smith wrote: > On Wed, Jan 6, 2016 at 3:34 PM, John McCall via cfe-commits > mailto:cfe-commits@lists.llvm.org>> wrote: > Author: rjmccall > Date: Wed Jan 6 17:34:20 2016 > New Revision: 256996 > > URL: http://llvm.org/viewv

r256996 - Properly bind up any cleanups in an ExprWithCleanups after

Author: rjmccall Date: Wed Jan 6 17:34:20 2016 New Revision: 256996 URL: http://llvm.org/viewvc/llvm-project?rev=256996&view=rev Log: Properly bind up any cleanups in an ExprWithCleanups after instantiating a default argument expression. This was previously just working implicitly by reinstantia

r256983 - Only instantiate a default argument once.

Author: rjmccall Date: Wed Jan 6 16:34:54 2016 New Revision: 256983 URL: http://llvm.org/viewvc/llvm-project?rev=256983&view=rev Log: Only instantiate a default argument once. By storing the instantiated expression back in the ParmVarDecl, we remove the last need for separately storing the sub-e

Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86

rjmccall added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:259 @@ +258,3 @@ +def err_anyx86_interrupt_attribute : Error< + "interrupt service routine %select{must have 'void' return value|" + "can only have a pointer argument and an optional integer a

Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86

rjmccall added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:259 @@ +258,3 @@ +def err_anyx86_interrupt_attribute : Error< + "interrupt service routine %select{must have 'void' return value|" + "can only have a pointer argument and an optional integer a

Re: [PATCH] D15647: [X86] Fix stack alignment for MCU target (Clang part)

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

Re: [PATCH] D15524: [GCC] Attribute ifunc support in clang

rjmccall added inline comments. Comment at: include/clang/AST/DeclBase.h:563 @@ +562,3 @@ + /// \brief Return true if this declaration is a definition of alias or ifunc. + bool hasDefiningAttr() const; + aaron.ballman wrote: > I think this function and getDefini

Re: [PATCH] D15524: [GCC] Attribute ifunc support in clang

rjmccall added a comment. This looks great, thanks. A few minor comment tweaks and this will be ready to commit. Comment at: include/clang/AST/DeclBase.h:562 @@ -561,1 +561,3 @@ + /// \brief Return true if this declaration is a definition of alias or ifunc. + bool hasDefin

Re: [PATCH] D15647: [X86] Fix stack alignment for MCU target (Clang part)

rjmccall added inline comments. Comment at: tools/clang/lib/AST/ASTContext.cpp:1902 @@ +1901,3 @@ + if (Target->getTriple().getArch() == llvm::Triple::xcore || + Target->getTriple().isOSIAMCU()) +return ABIAlign; // Never overalign on XCore and IAMCU. P

Re: [PATCH] D15709: [X86] Support 'interrupt' attribute for x86

rjmccall added inline comments. Comment at: include/clang/Basic/Attr.td:255 @@ -254,2 +254,3 @@ def TargetX86 : TargetArch<["x86"]>; +def TargetIA : TargetArch<["x86", "x86_64"]>; def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb"]> { As far as I'm

Re: [PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

rjmccall added a comment. Thank you, that's great. LGTM. http://reviews.llvm.org/D15674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

rjmccall added a comment. You don't need a CodeGenFunction for this; just use llvm::ConstantExpr::getBitCast. Please document the requirement that this returns an i8* in CGObjCRuntime.h. http://reviews.llvm.org/D15674 ___ cfe-commits mailing list

Re: [PATCH] D15524: [GCC] Attribute ifunc support in clang

rjmccall added inline comments. Comment at: include/clang/Basic/AttrDocs.td:1866 @@ +1865,3 @@ + let Content = [{ +The attribute ``__attribute__((ifunc("resolver")))`` is used to mark a function as an indirect function using the STT_GNU_IFUNC symbol type extension to the ELF st

Re: [PATCH] D15524: [GCC] Attribute ifunc support in clang

rjmccall added inline comments. Comment at: include/clang/Basic/AttrDocs.td:1866 @@ +1865,3 @@ + let Content = [{ +The attribute ``__attribute__((ifunc("resolver")))`` is used to mark a function as an indirect function using the STT_GNU_IFUNC symbol type extension to the ELF st

Re: [PATCH] D10599: [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

rjmccall added a comment. Richard should really be the one to sign off on this, since he's been managing the review so far. I am curious why we decided to handle this separately instead of treating it as a different attribute spelling kind. http://reviews.llvm.org/D10599 __

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

rjmccall added a comment. I'm sorry for the delay. It looks fine, thanks. http://reviews.llvm.org/D12614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r255531 - Allow pseudo-destructor calls on forward-declared Objective-C class pointers.

Author: rjmccall Date: Mon Dec 14 13:12:54 2015 New Revision: 255531 URL: http://llvm.org/viewvc/llvm-project?rev=255531&view=rev Log: Allow pseudo-destructor calls on forward-declared Objective-C class pointers. rdar://18522255 Added: cfe/trunk/test/SemaObjCXX/pseudo-destructor.mm Modified:

r255325 - Correctly type-check the default arguments of local functions

Author: rjmccall Date: Thu Dec 10 19:56:36 2015 New Revision: 255325 URL: http://llvm.org/viewvc/llvm-project?rev=255325&view=rev Log: Correctly type-check the default arguments of local functions when eagerly instantiating them. rdar://23721638 Modified: cfe/trunk/lib/Sema/SemaTemplateInsta

r255311 - In Objective-C, ignore attempts to redefine the ARC/GC qualifier macros.

Author: rjmccall Date: Thu Dec 10 17:31:01 2015 New Revision: 255311 URL: http://llvm.org/viewvc/llvm-project?rev=255311&view=rev Log: In Objective-C, ignore attempts to redefine the ARC/GC qualifier macros. This works around existing system headers which unconditionally redefine these macros. T

Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

rjmccall added inline comments. Comment at: lib/Driver/Tools.cpp:1477 @@ +1476,3 @@ + // If unspecified, choose the default based on the platform. + if (ABI == ppc::FloatABI::Invalid) { +ABI = ppc::FloatABI::Hard; hfinkel wrote: > rjmccall wrote: > > hfinkel

Re: [PATCH] D13351: [Power PC] add soft float support for ppc32

rjmccall added inline comments. Comment at: lib/Driver/Tools.cpp:1477 @@ +1476,3 @@ + // If unspecified, choose the default based on the platform. + if (ABI == ppc::FloatABI::Invalid) { +ABI = ppc::FloatABI::Hard; hfinkel wrote: > Don't need the { } here. We

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

rjmccall added a comment. Thanks! Just a slight tweak to the comment, then LGTM. Comment at: lib/Sema/SemaPseudoObject.cpp:259 @@ +258,3 @@ +/// +/// If this method returns false, and the set value isn't capturable for +/// some reason, the result of the expression

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 you

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. http://reviews.llvm

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? http://re

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:464 @@ -461,1 +463,3 @@ + if (useSetterResultAsExprResult(result.get())) +setResultToLastSemantic(); Sure, but that's not what I'm saying. I'm saying that the code, as you've writte

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 valueWithBy

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

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

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 warn_transparent_union_attribute_field_size_align : Warning< No, this diagnostic

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 b

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

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 specifies the alignment of the >> start of that

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* emitRoundPointerUpToAlignment(CodeGenF

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 storage

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

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 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(), +PSE->getRBracke

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

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 walked

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; @inte

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

rjmccall added a comment. By "in CGObjC", I mean you should be able to do it in CodeGenFunction::EmitObjCMessageExpr. Maybe put it in a separate function like static llvm::Value *tryGenerateSpecializedMessageSend(...) and then do something like } else if (llvm::Value *SpecializedResult =

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

rjmccall added a comment. Thanks, that's great. One minor tweak, and feel free to just commit when you've done that. Comment at: lib/CodeGen/CGCall.cpp:1420 @@ +1419,3 @@ + // If we have information about the function proto type, we can learn + // attributes form there. + A

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

rjmccall added inline comments. Comment at: lib/Sema/SemaPseudoObject.cpp:1627 @@ -1579,1 +1626,3 @@ +cast(Base->IgnoreParens())->getBaseExpr()); +return MSPropertyRefRebuilder(S, BaseOVE->getSourceExpr()).rebuild(E); } else { Hmm. Just like the Ob

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

rjmccall added a comment. That's exactly what I was looking for, thanks. Comment at: lib/CodeGen/CGExpr.cpp:3751 @@ +3750,3 @@ + // Preserve the function proto type because it contains useful information + // that we may be interested in using later on in the code generation.

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&view=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&view=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 la

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 CalleeIn

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 agree

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

rjmccall added a comment. In http://reviews.llvm.org/D14737#291481, @stephanemoore wrote: > I hope that it's not presumptuous of me to inquire but I was wondering if the > intent of this patch is to optimize calls to RR methods (and alloc) in > non-ARC code? Would I be correct in assuming that

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

rjmccall added inline comments. Comment at: include/clang/Basic/LangOptions.def:194 @@ -193,2 +193,3 @@ LANGOPT(ObjCAutoRefCount , 1, 0, "Objective-C automated reference counting") +LANGOPT(ObjCConvertMessagesToRuntimeCalls , 1, 0, "objc_* support for retain/release in t

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&view=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-we

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&view=rev Log: Remove -Wobjc-weak-compat; there isn't a compelling use case for this. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/S

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

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

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

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

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

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

<    3   4   5   6   7   8   9   >