[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-11 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:7291 + Address adjustAddrSpaceForAutoVar(Address A, const VarDecl *VD, + CodeGen::CodeGenFunction ) const override; }; yaxunl wrote: > rjmccall wrote: > > How about, instead of

[PATCH] D32977: [OpenCL] Emit function-scope variable in constant address space as static variable

2017-05-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:158 + // variable in constant address space in OpenCL. + if (D.getStorageDuration() != SD_Automatic && !D.hasExternalStorage()) { llvm::GlobalValue::LinkageTypes Linkage = Please rearrange

[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-05-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. The right fix is probably just to make sure that EmitLValueForField doesn't add TBAA information when the base LValue doesn't have it. That will also fix problems with recursive member access where the base is may_alias. Repository: rL LLVM

[PATCH] D33284: [CodeGen] Propagate LValueBaseInfo instead of AlignmentSource

2017-05-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Generally looks great, thanks! Comment at: lib/CodeGen/CGBlocks.cpp:923 + MakeAddrLValue(blockField, type, + LValueBaseInfo(AlignmentSource::Decl, false)), /*captured by init*/ false);

[PATCH] D33284: [CodeGen] Propagate LValueBaseInfo instead of AlignmentSource

2017-05-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:2256 Address(CapLVal.getPointer(), getContext().getDeclAlign(VD)), -CapLVal.getType(), AlignmentSource::Decl); +CapLVal.getType(), LValueBaseInfo(AlignmentSource::Decl,

[PATCH] D33284: [CodeGen] Propagate LValueBaseInfo instead of AlignmentSource

2017-05-17 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D33284#757917, @kparzysz wrote: > Changed the getNatural.*TypeAlignment to produce the entire LValueBaseInfo, > assuming MayAlias to be false. > > Added merging of LValueBaseInfos. The merging assumes that the alignment > source in the

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaDecl.cpp:7210 + return; +} } err_opencl_function_variable seems like a better diagnostic, at least for opencl_global. You can fall back on this more general diagnostic for other address

[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-05-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D31885#755456, @kparzysz wrote: > In https://reviews.llvm.org/D31885#751164, @rjmccall wrote: > > > The right fix is probably just to make sure that EmitLValueForField doesn't > > add TBAA information when the base LValue doesn't have it.

[PATCH] D32977: [OpenCL] Emit function-scope variable in constant address space as static variable

2017-05-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaDecl.cpp:10286 + // these variables must be a compile time constant. + VDecl->getType().getAddressSpace() == LangAS::opencl_constant) CheckForConstantInitializer(Init, DclT);

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Looking good. We had some overlapping patches and comments there, so I want to make sure you didn't miss my comment on EmitAutoVarAlloca. Comment at: lib/Sema/SemaDecl.cpp:7205 +} +assert (T.getAddressSpace() != LangAS::opencl_constant); +

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1115 + assert(T.getAddressSpace() == LangAS::Default || + T.getQualifiers().hasTargetSpecificAddressSpace()); + auto Addr = getTargetHooks().performAddrSpaceCast(*this, yaxunl wrote: >

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:7296 + unsigned getASTAllocaAddressSpace() const override { +return LangAS::Count + getABIInfo().getDataLayout().getAllocaAddrSpace(); + } yaxunl wrote: > rjmccall wrote: > > Can we

[PATCH] D32977: [OpenCL] Emit function-scope variable in constant address space as static variable

2017-05-13 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaDecl.cpp:10286 + // these variables must be a compile time constant. + VDecl->getType().getAddressSpace() == LangAS::opencl_constant) CheckForConstantInitializer(Init, DclT);

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:1120 +address.getPointer()->getType()->getPointerElementType()->getPointerTo( +getContext().getTargetAddressSpace(T.getAddressSpace())), +/*non-null*/ true); A lot of

[PATCH] D32977: [OpenCL] Emit function-scope variable in constant address space as static variable

2017-05-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaDecl.cpp:7129 +NewVD->getType().getAddressSpace() != LangAS::opencl_constant) || NewVD->hasExternalStorage()) { if (!T->isSamplerT() && Seeing criteria like this, and a lot of the

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-10 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:7291 + Address adjustAddrSpaceForAutoVar(Address A, const VarDecl *VD, + CodeGen::CodeGenFunction ) const override; }; How about, instead of introducing a second method, we just change

[PATCH] D17143: [Sema] PR25156 Crash when parsing dtor call on incomplete type

2017-06-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:192 Found.clear(); -if (Step == 0 && LookupCtx) +if (Step == 0 && LookupCtx && !RequireCompleteDeclContext(SS, LookupCtx)) LookupQualifiedName(Found, LookupCtx); You should

[PATCH] D28691: Add OpenCL 2.0 atomic builtin functions as Clang builtin

2017-06-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Looks like you haven't introduced proper constants in the header for scopes. Comment at: docs/LanguageExtensions.rst:1855 +atomic builtins are in ``explicit`` form of the corresponding OpenCL 2.0 +builtin function, and are named with a ``__opencl__``

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

2017-06-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1605 + ? CGM.getDataLayout().getAllocaAddrSpace() + : getContext().getTargetAddressSpace(LangAS::Default)); break; yaxunl wrote: > rjmccall wrote: > > Everything

[PATCH] D34198: Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.

2017-06-21 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:4128 +return true; +} + puneetha wrote: > rjmccall wrote: > > I don't understand the difference you're creating between traits here. > > Three specific traits about

[PATCH] D34454: SwiftCC: Perform physical layout when computing coercion types

2017-06-21 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Looks great, thanks! https://reviews.llvm.org/D34454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33842: [AMDGPU] Fix address space of global variable

2017-06-22 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/Basic/TargetInfo.h:969 + /// LangAS::Default. + virtual unsigned getGlobalAddressSpace() const { return LangAS::Default; } + I'm not sure this really needs to exist. Comment at:

[PATCH] D34198: Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.

2017-06-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:4128 +return true; +} + I don't understand the difference you're creating between traits here. Three specific traits about destructibility allow incomplete array types regardless

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

2017-06-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:1605 + ? CGM.getDataLayout().getAllocaAddrSpace() + : getContext().getTargetAddressSpace(LangAS::Default)); break; Everything about your reasoning seems to apply

[PATCH] D33842: [AMDGPU] Fix address space of global variable

2017-06-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/Basic/TargetInfo.h:959 + /// \brief Return the target address space which is read only and can be + /// casted to the generic address space. + virtual llvm::Optional getTargetConstantAddressSpace() const {

[PATCH] D33284: [CodeGen] Propagate LValueBaseInfo instead of AlignmentSource

2017-05-18 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Yes, of course. Although "looks good to me" is acceptance whether or not I actually push a button in Phabricator. :) Repository: rL LLVM https://reviews.llvm.org/D33284

[PATCH] D33328: [CodeGen] Pessimize aliasing for union members (and may-alias) objects

2017-05-24 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:1436 +if (BaseInfo.getMayAlias()) + TBAAInfo = CGM.getTBAAInfo(getContext().CharTy); llvm::MDNode *TBAAPath = CGM.getTBAAStructTagInfo(TBAABaseType, TBAAInfo, kparzysz wrote: >

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-05-26 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:3517 +CGM.getCodeGenOpts().StrictVTablePointers && +CGM.getCodeGenOpts().OptimizationLevel > 0) + addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),

[PATCH] D33437: Emit available_externally vtables opportunistically

2017-05-26 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/AST/VTableBuilder.h:160 + "GlobalDecl can be created only from virtual function"); +if (getKind() == CK_FunctionPointer) + return GlobalDecl(getFunctionDecl()); Please use an exhaustive

[PATCH] D33328: [CodeGen] Pessimize aliasing for union members (and may-alias) objects

2017-05-24 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, that looks great. Repository: rL LLVM https://reviews.llvm.org/D33328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32401: [Devirtualization] insert placement new barrier with -O0

2017-05-19 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D32401#759223, @Prazek wrote: > Is everyone ok with sending this patch? As long as this continues to be an opt-in optimization, go ahead and do your research project, I suppose. https://reviews.llvm.org/D32401

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:7296 + unsigned getASTAllocaAddressSpace() const override { +return LangAS::Count + getABIInfo().getDataLayout().getAllocaAddrSpace(); + } yaxunl wrote: > rjmccall wrote: > > yaxunl

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:411 +CodeGen::CodeGenFunction , llvm::Value *Src, unsigned SrcAddr, +unsigned DestAddr, llvm::Type *DestTy, bool isNonNull) const { // Since target may map different address spaces in AST to the

[PATCH] D32248: CodeGen: Cast alloca to expected address space

2017-05-16 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Looks great, thank you for being patient. https://reviews.llvm.org/D32248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D33437: Emit available_externally vtables opportunistically

2017-05-28 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/AST/VTableBuilder.h:160 + "GlobalDecl can be created only from virtual function"); +if (getKind() == CK_FunctionPointer) + return GlobalDecl(getFunctionDecl()); Prazek wrote: > rjmccall

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-05-28 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Looks great, thanks. One request for additional tests, but feel free to commit when that's ready. Comment at: test/CodeGenCXX/strict-vtable-pointers.cpp:258 +

[PATCH] D33706: CodeGen: Cast temporary variable to proper address space

2017-06-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.cpp:3805 +Address Addr = +CreateMemTemp(I->Ty, ArgInfo.getIndirectAlign(), "tmp", false); IRCallArgs[FirstIRArg] = Addr.getPointer(); How about "indirect-arg-temp" as the

[PATCH] D33698: [CodeGen][ObjC] Fix assertion failure in CodeGenFunction::EmitARCStoreStrongCall

2017-05-31 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D33698 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D33437: Emit available_externally vtables opportunistically

2017-05-31 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Looks great, thanks! https://reviews.llvm.org/D33437 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D33706: [AMDGPU] Fix address space for global and temporary variables in C++

2017-05-31 Thread John McCall via Phabricator via cfe-commits
rjmccall requested changes to this revision. rjmccall added a comment. This revision now requires changes to proceed. Please split this up into two patches: - one for your target-specific changes to the global address space and so on - one for the generic fix to CreateTempAlloca etc.

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-05-31 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: test/CodeGenCXX/strict-vtable-pointers.cpp:263 + +struct VirtualInVBase : virtual Empty, HoldingVirtuals { +}; "virtual" applies to an individual base, not to all following bases. You need "virtual HoldingVirtuals"

[PATCH] D33706: CodeGen: Cast temporary variable to proper address space

2017-06-02 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks! https://reviews.llvm.org/D33706 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

2017-06-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/AST/Decl.h:1388 SourceLocation IdLoc, IdentifierInfo *Id, - QualType T); + QualType T, bool IsThisOrSelf = false);

[PATCH] D33706: CodeGen: Cast temporary variable to proper address space

2017-06-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Thanks. One tweak and then LGTM. Comment at: lib/CodeGen/CodeGenFunction.h:1937 + /// to the stack. + + /// Because the address of a temporary is often exposed to the program in This line should have ///.

[PATCH] D33842: [AMDGPU] Fix address space of global variable

2017-06-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:360 + CGF.getContext().getTargetAddressSpace(LangAS::opencl_constant); +} auto *GV = new llvm::GlobalVariable( This is not an appropriate place to stick

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

2017-06-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/AST/Decl.h:1387 +IPK_CapturedContext, /// Parameter for captured context +IPK_GeneralParam,/// General implicit parameter + }; ABataev wrote: > rjmccall wrote: > > I would just call this

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

2017-06-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Sure thing, r304951. https://reviews.llvm.org/D17215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-06-01 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. Thanks, LGTM. https://reviews.llvm.org/D31830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

2017-06-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D33735#770318, @aaron.ballman wrote: > In https://reviews.llvm.org/D33735#770296, @ABataev wrote: > > > In https://reviews.llvm.org/D33735#770288, @aaron.ballman wrote: > > > > > Can you help me to understand what problem is being solved with

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

2017-06-08 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added inline comments. Comment at: include/clang/AST/Decl.h:901 +/// member functions. +unsigned ImplicitParamKind : 3; }; aaron.ballman wrote: > rjmccall wrote: > > ABataev wrote: > > > aaron.ballman wrote: >

[PATCH] D17143: [Sema] PR25156 Crash when parsing dtor call on incomplete type

2017-06-10 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, looks great. If you're going to be submitting multiple patches, you should really ask for commit access; it's not an arduous process. https://reviews.llvm.org/D17143

[PATCH] D34198: Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.

2017-06-14 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Test cases? https://reviews.llvm.org/D34198 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

2017-05-01 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I agree with Aaron, it would be nice if this had some immediate effect. One obvious suggestion: take advantage of it in code generation. LLVM already has a parameter attribute called "nocapture" which conveys exactly the right semantics for noescape pointers and

[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-05-08 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D31885#748873, @kparzysz wrote: > Ping. > > What's the next step here? Sounds to me like we should just not apply struct-path TBAA data that runs through a union field because either LLVM's representation can't handle it or Clang isn't

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

2017-06-05 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, looks great. Do you still not have commit access? https://reviews.llvm.org/D17215 ___ cfe-commits mailing list

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

2017-06-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:8985 +// FIXME: Would this be considered an almost match as well? +continue; } These continues that you're adding are already at the end of the loop body, so this code

[PATCH] D33735: [DebugInfo] Add ThisOrSelf attribute for emission of FlagObjectPointer.

2017-06-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/AST/Decl.h:1387 +IPK_CapturedContext, /// Parameter for captured context +IPK_GeneralParam,/// General implicit parameter + }; I would just call this "Other" and document it as being for

[PATCH] D38118: [CodeGen][ObjC] Build the global block structure before emitting the body of global block invoke functions

2017-09-21 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM with one tweak. Comment at: lib/CodeGen/CGBlocks.cpp:1124 LocalDeclMap, -

[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

2017-09-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Thanks, that looks good. https://reviews.llvm.org/D32210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38074: Fix TBAA information for reference accesses

2017-09-22 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Sure, that's fine. LGTM. https://reviews.llvm.org/D38074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Are you sure it's a good idea to not print the address space when it's implicit? Won't that often lead to really confusing diagnostics? Also, we do already have a way of expressing that an extended qualifier was explicit: AttributedType. We have very similar sorts

[PATCH] D37826: Refine generation of TBAA information in clang

2017-10-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D37826#888086, @kosarev wrote: > In https://reviews.llvm.org/D37826#887820, @rjmccall wrote: > > > I assume I should wait on reviewing this until all of these smaller TBAA > > patches land? > > > These small patches are actually part of this

[PATCH] D38503: [CodeGen] Unify generation of scalar and struct-path TBAA tags

2017-10-04 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM with one comment fix that I noticed. Comment at: lib/CodeGen/CodeGenModule.h:659 - llvm::MDNode *getTBAAInfoForVTablePtr(); + /// getTBAAAccessInfo - Get TBAA

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

2017-10-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D38473#888597, @rsmith wrote: > In https://reviews.llvm.org/D38473#888159, @mppf wrote: > > > > You should also indicate *which* record layout (complete object type or > > > base subobject type) the field number is for. I don't think there's

[PATCH] D31363: [libc++] Remove cmake glob for source files

2017-10-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a reviewer: dexonsmith. rjmccall added a comment. I'll let Duncan answer that question. https://reviews.llvm.org/D31363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D35082#890359, @Anastasia wrote: > In https://reviews.llvm.org/D35082#890162, @rjmccall wrote: > > > Okay. I think I see your point about why it would be better to have a > > canonical __private address space that is different from the

[PATCH] D38659: [Sema][ObjC] Preserve syntactic sugar when removing ARCReclaimReturnedObject cast

2017-10-06 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D38659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37826: Refine generation of TBAA information in clang

2017-10-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I assume I should wait on reviewing this until all of these smaller TBAA patches land? https://reviews.llvm.org/D37826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

2017-10-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: include/clang/CodeGen/CodeGenABITypes.h:81 +// Returns a field number for a struct suitable for GEP'ing +unsigned getFieldNumber(CodeGenModule , "Given a non-bitfield struct field, return its index within the

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Why is most of this patch necessary under the design of adding a non-canonical __private address space? Comment at: include/clang/AST/Type.h:336 + /// space makes difference. + bool getImplicitAddressSpaceFlag() const { return Mask & IMask; } +

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Non-canonical information is not supposed to be mangled. It's not clear to me what the language rule you're really trying to implement is. Maybe you really do need a canonical __private address space, in which case you are going to have to change a lot of code in

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. You have an important backend change relying on being able to preserve type sugar better in diagnostics? The only apparent semantic change in this patch is that you're changing the mangling, which frankly seems incorrect. https://reviews.llvm.org/D35082

[PATCH] D37826: Refine generation of TBAA information in clang

2017-10-05 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D37826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38606: [CodeGen] Emit a helper function for __builtin_os_log_format to reduce code size

2017-10-06 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks. LGTM. https://reviews.llvm.org/D38606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Okay. I think I see your point about why it would be better to have a canonical __private address space that is different from the implicit address space 0. I assume this means that there should basically never be pointers, references, or l-values in address space 0

[PATCH] D38947: [CodeGen] Refine generation of TBAA info for bit-field lvalues

2017-10-17 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay, thanks. https://reviews.llvm.org/D38947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D35082#895312, @yaxunl wrote: > Thanks. I will separate the implicit addr space flag to another patch. Thanks, appreciated. Comment at: include/clang/AST/Type.h:562 + static const uint32_t IMask = 0x200; + static const

[PATCH] D35082: [OpenCL] Add LangAS::opencl_private to represent private address space in AST

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. A few minor comments; feel free to commit after addressing them. Comment at: include/clang/Basic/AddressSpaces.h:34 // OpenCL specific address spaces. opencl_global, yaxunl wrote: > rjmccall wrote: > > I think you need a real

[PATCH] D38861: [CodeGen] Error on unsupported checked multiplies early

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:2263 + } +} + Is there a reason this only fails on x86? If LLVM doesn't have generic wide-operation lowering, this probably needs to be a target whitelist rather than a blacklist.

[PATCH] D38788: [CodeGen] EmitCXXMemberDataPointerAddress() to generate TBAA info along with LValue base info

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D38788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38791: [CodeGen] EmitLoadOfPointer() to generate TBAA info along with LValue base info

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D38791 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38793: [CodeGen] EmitLoadOfReference() to generate TBAA info along with LValue base info

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D38793 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38795: [CodeGen] emitOMPArraySectionBase() to generate TBAA info along with LValue base info

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D38795 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38796: [CodeGen] EmitPointerWithAlignment() to generate TBAA info along with LValue base info

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Yes, thank for you handling the cast case well. Repository: rL LLVM https://reviews.llvm.org/D38796 ___ cfe-commits mailing list

[PATCH] D38794: [CodeGen] getNaturalTypeAlignment() to generate TBAA info along with LValue base info

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. This function feels increasingly poorly-named, but let's leave that alone for now. Repository: rL LLVM https://reviews.llvm.org/D38794

[PATCH] D38773: [Sema] Add support for flexible array members in Obj-C.

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5226 +def err_objc_variable_sized_type_not_at_end : Error< + "field %0 with variable sized type %1 is not at the end of class">; +def note_next_field_declaration : Note<

[PATCH] D38861: [CodeGen] Error on unsupported checked multiplies early

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Okay. Sounds good to me. We intend to actually implement the generic lowering, I hope? https://reviews.llvm.org/D38861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38773: [Sema] Add support for flexible array members in Obj-C.

2017-10-12 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15055 } + // If it is the last field is checked elsewhere. } vsapsai wrote: > rjmccall wrote: > > "Whether" rather than "If", please. You should also leave a

[PATCH] D38945: [CodeGen] Pass TBAA info along with lvalue base info everywhere

2017-10-16 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D38945 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38796: [CodeGen] EmitPointerWithAlignment() to generate TBAA info along with LValue base info

2017-10-16 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Ok. https://reviews.llvm.org/D38796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38966: CodeGen: Fix invalid bitcasts for atomic builtins

2017-10-16 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D38966 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38947: [CodeGen] Refine generation of TBAA info for bit-field lvalues

2017-10-16 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:3665 -LValue CodeGenFunction::EmitLValueForField(LValue base, - const FieldDecl *field) { - LValueBaseInfo BaseInfo = base.getBaseInfo(); - AlignmentSource

[PATCH] D34367: CodeGen: Fix address space of indirect function argument

2017-09-09 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/CodeGen/CGCall.h:211 + push_back(CallArg(rvalue, type, needscopy, AS)); } Ah, I think I understand what's going on here now. "indirect byval" arguments include an implicit copy to the stack, and the

[PATCH] D33826: [clang-tidy] avoid pointer cast to more strict alignment check

2017-08-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In https://reviews.llvm.org/D33826#856652, @lebedev.ri wrote: > In https://reviews.llvm.org/D33826#856619, @Eugene.Zelenko wrote: > > > In https://reviews.llvm.org/D33826#856610, @lebedev.ri wrote: > > > > > Any status update here? :) > > > I generally do see a benefit

[PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom used by glibc

2017-08-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8808 +Context.getTypeSize(pointerType) == +Context.getTypeSize(IExp->getType())) + IsGNUIdiom = true; efriedma wrote: > andrew.w.kaylor wrote: > > efriedma wrote: > > >

[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8589 + Expr *LHS = E->getLHS()->IgnoreParenImpCasts(); + Expr *RHS = E->getRHS()->IgnoreParenImpCasts(); + lebedev.ri wrote: > rjmccall wrote: > > Do you still need these? I'm always antsy

[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8879 + if (IsComparisonConstant) return AnalyzeImpConvsInComparison(S, E); lebedev.ri wrote: > rjmccall wrote: > > Part of the purpose of checking for signed comparisons up here is

[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8592 + bool BadR = !RType->isIntegerType() || RType->isSignedIntegerType() || + RHS->isKnownToHaveBooleanValue(); + Please extract a function which computes this for an Expr* and

[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Great, thanks. Just a few tweaks. Comment at: docs/ReleaseNotes.rst:76 + ``0`` constant was adjusted to warn regardless of whether the constant is + signed or unsigned. + "now warns when comparing an unsigned integer and 0

[PATCH] D37565: [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

2017-09-07 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Comment at: docs/ReleaseNotes.rst:79 +- ``-Wtautological-compare`` now warns about comparison of signed integer and + ``0U`` constant when appropriate. +

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