[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext

2018-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. This seems like a patch with a good direction, I generally think that this change doesn't change the readability of the code (and generally matches the structure of other code in clang). It is generally a mechanical change, though I'd suggest running clang-format (

[PATCH] D49732: [AST][2/4] Move the bit-fields from FunctionDecl and CXXConstructorDecl into DeclContext

2018-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Anotherone in need of clang-format, otherwise seems alright to me. Comment at: include/clang/AST/Decl.h:1809 + // and setMultiVersion do not simply set/read the corresponding bit. + void setPureImpl(bool isPure) { FunctionDeclBits.IsPure = isPure;

[PATCH] D49733: [AST][3/4] Move the bit-fields from BlockDecl, LinkageSpecDecl and OMPDeclareReductionDecl into DeclContext

2018-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Another one in need of clang-format, but generally seems pretty mechanical. Comment at: include/clang/AST/DeclCXX.h:2844 + + bool hasBracesImpl() const { return LinkageSpecDeclBits.HasBraces; } + void setBracesImpl(bool B = true) { LinkageSpecDeclB

[PATCH] D49734: [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext

2018-07-24 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Same comment as the other 3. Comment at: include/clang/AST/DeclObjC.h:190 + /// InvalidObjCMethodFamily cast into an ObjCMethodFamily. + ObjCMethodFamily getFamilyImpl() const { +return static_cast(ObjCMethodDeclBits.Family); I

[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext

2018-07-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. This looks acceptable to me. I'd like to get @rsmith or @rnk to approve the approach though. Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D49838: [AST] Sink 'part of explicit cast' down into ImplicitCastExpr

2018-07-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. 2 small items, otherwise looks good. Comment at: include/clang/AST/Expr.h:2824 CastExprBits.Kind = kind; -CastExprBits.PartOfExplicitCast = false; setBasePathSize(BasePathSize); So, I'd prefer that this line get left in

[PATCH] D49844: [AST] Add a isActuallyImplicitCast() helper to the CastExpr class.

2018-07-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. I'm not sure that this logic requires a separate function. Since you've fixed the getIsPartOfExplicitCast logic correctly, it is pretty simple... Comment at: include/clang/AST/Expr.h:2854 + /// There are two global types of casts - implicit and ex

[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

2018-07-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. 1 Nit, otherwise LGTM. Comment at: docs/UndefinedBehaviorSanitizer.rst:95 + of bigger bit width to smaller bit width, if that results in data loss. + That is, if the demoted value, after casting back to the

[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext

2018-07-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: include/clang/AST/Decl.h: + + /// True if this is a C++11 scoped enumeration. + void setScopedUsingClassTag(bool ScopedUCT = true) { This is the same comment as 3330, perhaps a copy/paste error? ==

[PATCH] D49844: [AST] Add a isActuallyImplicitCast() helper to the CastExpr class.

2018-07-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. If @rsmith requested this, I'm fine with it then. Repository: rC Clang https://reviews.llvm.org/D49844 ___ cfe-commits mailing list cf

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

2018-07-31 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Its not that it needs to be 'at least 9 bits', IMO 9 bits was too small as well. What I'd prefer to this solution is similar to what John McCall said on PR38356: Add the size to the trailing allocated space. Check out the 'create' methods for all of these, and make

[PATCH] D50056: [NFC] Silence warning about ptr-to-func to ptr-to-obj cast in clang-fuzzer/handle-llvm/handle_llvm.cpp.

2018-07-31 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:152 typedef void (*func)(int*, int*, int*, int); - func f = reinterpret_cast(EE->getPointerToFunction(EntryFunc)); +#if defined(__GNUC__) && ((__GNUC__ == 4) && (__GNUC_MINOR__

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

2018-07-31 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: include/clang/AST/Expr.h:2791 +public: + using BasePathSizeTy = unsigned short; + static_assert(std::numeric_limits::max() >= 16384, I'll let @rjmccall comment here, but I think I'd be willing to give up 2 more byt

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

2018-07-31 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: include/clang/AST/Expr.h:2805 + } + BasePathSizeTy &BasePathSize(); + erichkeane wrote: > Why is this a reference? This seems odd... It seems that the const-cast > magic above shouldn't be necessary either. Lookin

[PATCH] D50056: [NFC] Silence warning about ptr-to-func to ptr-to-obj cast in clang-fuzzer/handle-llvm/handle_llvm.cpp.

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. LMK if you need someone to commit this for you. https://reviews.llvm.org/D50056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. LGTM! Repository: rC Clang https://reviews.llvm.org/D49729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48100: Append new attributes to the end of an AttributeList.

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: test/Sema/attr-ownership.c:22 void f15(int, int) - __attribute__((ownership_returns(foo, 1))) // expected-note {{declared with index 1 here}} - __attribute__((ownership_returns(foo, 2))); // expected-error {{'ownership_returns'

[PATCH] D49729: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into DeclContext

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC338630: [AST][1/4] Move the bit-fields from TagDecl, EnumDecl and RecordDecl into… (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D49729?vs=158498&id=158624#t

[PATCH] D49732: [AST][2/4] Move the bit-fields from FunctionDecl and CXXConstructorDecl into DeclContext

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC338636: [AST][2/4] Move the bit-fields from FunctionDecl and CXXConstructorDecl into… (authored by erichkeane, committed by ). Repository: rC Clang https://reviews.llvm.org/D49732 Files: include/cla

[PATCH] D49733: [AST][3/4] Move the bit-fields from BlockDecl, LinkageSpecDecl and OMPDeclareReductionDecl into DeclContext

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC338639: [AST][3/4] Move the bit-fields from BlockDecl, LinkageSpecDecl and… (authored by erichkeane, committed by ). Repository: rC Clang https://reviews.llvm.org/D49733 Files: include/clang/AST/Dec

[PATCH] D49734: [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into DeclContext

2018-08-01 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL338641: [AST][4/4] Move the bit-fields from ObjCMethodDecl and ObjCContainerDecl into… (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://

[PATCH] D48100: Append new attributes to the end of an AttributeList.

2018-08-02 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In https://reviews.llvm.org/D48100#1185189, @hfinkel wrote: > Assuming that @aaron.ballman is still okay with this, I think that we should > move forward. @erichkeane, I suspect that fixing the ordering problems, which > were often arbitrary before and are still so,

[PATCH] D50163: [AST] Remove the static_assert check in ObjCMethodDecl::ObjCMethodDecl

2018-08-02 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. @bricci : This likely needs to be moved into the bitfield section I think. We had a couple of other static-asserts that moved, so this is likely one of them. Repository: rC Clang https://reviews.llvm.org/D50163 ___ c

[PATCH] D48100: Append new attributes to the end of an AttributeList.

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In https://reviews.llvm.org/D48100#1186654, @Meinersbur wrote: > I have two approaches to tackle the wrong marker order: > https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO > both are too invasive to be justified for the small issue. I think

[PATCH] D50214: Add inherited attributes before parsed attributes.

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. IMO, I think this (and the next 2 numerically), show to me that we have too much dependence on the order of attributes, which all of these show is not a reliable thing to count on. I quite dislike this as well, and think this is going to lead us to chasing these sam

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1008 + // We ignore conversions to/from pointer and/or bool. + if (!(SrcType->isIntegerType() && DstType->isIntegerType())) +return; I'd rather !SrcType->isInt || !DestType->isInt

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:2004 +if (auto *ICE = dyn_cast(CE)) { + if (CGF.SanOpts.hasOneOf(SanitizerKind::ImplicitConversion) && + !ICE->isPartOfExplicitCast()) { lebedev.ri wrote: > erichkeane wr

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1008 + // We ignore conversions to/from pointer and/or bool. + if (!(SrcType->isIntegerType() && DstType->isIntegerType())) +return; lebedev.ri wrote: > erichkeane wrote: > > I'd rat

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. On the face, this seems to be a nice patch. Removing extra allocations and replacing them with the stack is a good thing IMO. This is clearly an example of PIMPL, which while it has its advantages, performance is typically a hit. My 2 concerns with this are: 1- Th

[PATCH] D50261: [AST] Remove unnecessary indirections in DeclarationNameTable

2018-08-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In https://reviews.llvm.org/D50261#1187705, @bricci wrote: > It seems to me that the increased size of DeclarationNameTable is irrelevant > since it is > only used as a member in ASTContext and never copied nor moved from. Also, > at least for C++ it seems to me tha

[PATCH] D40218: [Clang] Add __builtin_launder

2018-08-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Bump! I don't see anything here that seems questionable (and this has extensive testing), but I presume @rsmith should be the final one to approve. https://reviews.llvm.org/D40218 ___ cfe-commits mailing list cfe-commit

[PATCH] D42758: Support `#pragma comment(lib, "name")` in the frontend for ELF

2018-02-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Just 1 format question, otherwise Looks good. Comment at: docs/LanguageExtensions.rst:2732 + +The ``#pragma comment(lib, ...)`` directive is supported on all ELF targ

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values

2018-02-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. LLVM patch here: https://reviews.llvm.org/D42979 Repository: rC Clang https://reviews.llvm.org/D42978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values

2018-02-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: aaron.ballman, craig.topper, echristo. Herald added subscribers: fedor.sergeev, kbarton, aheejin, kristof.beyls, jgravelle-google, sbc100, javed.absar, nhaehnle, nemanjai, sdardis, dylanmckay, jyknight, dschuff, jfb, aemerson, jholewin

[PATCH] D43041: Add X86 Support to ValidCPUList (enabling march notes)

2018-02-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: samparker, fhahn, rengolin, echristo, craig.topper. A followup to: https://reviews.llvm.org/D42978 This patch adds X86 and X86_64 support for enabling the march notes. Repository: rC Clang https://reviews.llvm.org/D43041 Files:

[PATCH] D43045: Add NVPTX Support to ValidCPUList (enabling march notes)

2018-02-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: samparker, fhahn, rengolin, echristo, Hahnfeld. Herald added a subscriber: jholewinski. A followup to: https://reviews.llvm.org/D42978 This patch adds NVPTX support for enabling the march notes. Repository: rC Clang https://reviews

[PATCH] D43057: Add Rest of Targets Support to ValidCPUList (enabling march notes)

2018-02-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: samparker, fhahn, rengolin, echristo, craig.topper. Herald added subscribers: fedor.sergeev, kbarton, aheejin, jgravelle-google, sbc100, nhaehnle, nemanjai, sdardis, dylanmckay, jyknight, dschuff, jfb. A followup to: https://reviews.ll

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133276. erichkeane retitled this revision from "Make march/target-cpu print a note with the list of valid values" to "Make march/target-cpu print a note with the list of valid values for ARM". erichkeane edited the summary of this revision. https://review

[PATCH] D43041: Add X86 Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133443. erichkeane added a comment. removed careless newline, also rebases off ARM patch. https://reviews.llvm.org/D43041 Files: lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h test/Misc/target-invalid-cpu-note.c Index: test/Misc/target-invalid

[PATCH] D43041: Add X86 Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133456. erichkeane added a comment. Cleaned up the test as suggested. https://reviews.llvm.org/D43041 Files: lib/Basic/Targets/X86.cpp lib/Basic/Targets/X86.h test/Misc/target-invalid-cpu-note.c Index: test/Misc/target-invalid-cpu-note.c

[PATCH] D43045: Add NVPTX Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133458. erichkeane added a comment. rebased, reflowed tests. https://reviews.llvm.org/D43045 Files: include/clang/Basic/Cuda.h lib/Basic/Cuda.cpp lib/Basic/Targets/NVPTX.cpp lib/Basic/Targets/NVPTX.h test/Misc/target-invalid-cpu-note.c Index:

[PATCH] D43057: Add Rest of Targets Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133459. erichkeane added a comment. rebased, reflowed tests. https://reviews.llvm.org/D43057 Files: lib/Basic/Targets/AMDGPU.cpp lib/Basic/Targets/AMDGPU.h lib/Basic/Targets/AVR.cpp lib/Basic/Targets/AVR.h lib/Basic/Targets/BPF.cpp lib/Basic/

[PATCH] D43045: Add NVPTX Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: test/Misc/target-invalid-cpu-note.c:38 +// NVPTX: note: valid target CPU values are: sm_20, sm_21, sm_30, sm_32, sm_35, +// NVPTX-SAME: sm_37, sm_50, sm_52, sm_53, sm_60, sm_61, sm_62, sm_70, sm_72 tra wrote: > Nit: G

[PATCH] D43041: Add X86 Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324674: Add X86 Support to ValidCPUList (enabling march notes) (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43041?

[PATCH] D43045: Add NVPTX Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324675: Add NVPTX Support to ValidCPUList (enabling march notes) (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4304

[PATCH] D43057: Add Rest of Targets Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC324676: Add Rest of Targets Support to ValidCPUList (enabling march notes) (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D43057?vs=133459&id=133509#toc Repo

[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: echristo, craig.topper, aaron.ballman. Herald added a subscriber: mgrang. As reported here: https://bugs.llvm.org/show_bug.cgi?id=36301 The issue is that the 'use' causes the plain declaration to emit the attributes to LLVM-IR. However,

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133442. erichkeane marked an inline comment as done. erichkeane added a comment. Removed extra newline. https://reviews.llvm.org/D42978 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. That makes sense. I changed the ARM one to check ONLY the first one, since in each case that seems like the oldest anyway. It at least makes sure that the lists are distinct, and avoids this cross-repo fragility. https://reviews.llvm.org/D42978

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133435. erichkeane added a comment. Simplified ARM tests as requested. https://reviews.llvm.org/D42978 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/Basic/Targets/AArch64.cpp lib/

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: test/Misc/target-invalid-cpu-note.c:1 +// RUN: not %clang_cc1 -triple armv5--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix ARM +// ARM: error: unknown target CPU 'not-a-cpu' Hahnfeld wro

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: test/Misc/target-invalid-cpu-note.c:1 +// RUN: not %clang_cc1 -triple armv5--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix ARM +// ARM: error: unknown target CPU 'not-a-cpu' erichkeane w

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 133452. erichkeane added a comment. Relaxed the test further. https://reviews.llvm.org/D42978 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/Basic/Targets/AArch64.cpp lib/Basic/Tar

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-02-08 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL324673: Make march/target-cpu print a note with the list of valid values for ARM (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

2018-02-12 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC324907: Make attribute-target on a Definition-after-use update the LLVM attributes (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D43095?vs=133519&id=133880#t

[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

2018-02-12 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 3 inline comments as done. erichkeane added inline comments. Comment at: lib/CodeGen/CodeGenModule.cpp:1318-1325 +llvm::AttrBuilder Attrs; +if (GetCPUAndFeaturesAttributes(D, Attrs)) { + // We know that GetCPUAndFeaturesAttributes will a

[PATCH] D40925: Add option -fkeep-static-consts

2018-02-13 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. >> OK. My concern is that users need to use __attribute__((used)) or something >> more robust if they want SVN identifiers to reliably appear in the output. >> Adding this flag just creates a trap that will fail once they turn on >>-O2. >> I'd rather not have it in t

[PATCH] D43259: Implement function attribute artificial

2018-02-13 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL325081: Implement function attribute artificial (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43259?vs=134123&id=13

[PATCH] D43321: Improve documentation for attribute artificial

2018-02-14 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC325186: Improve documentation for attribute artificial (authored by erichkeane, committed by ). Repository: rC Clang https://reviews.llvm.org/D43321 Files: include/clang/Basic/AttrDocs.td Index: i

[PATCH] D43359: Clean up 'target' attribute diagnostics

2018-02-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: echristo, aaron.ballman. There were a few issues previously with the target attribute diagnostics implementation that lead to the attribute being added to the AST despite having an error in it. This patch changes that, and adds a test

[PATCH] D43359: Clean up 'target' attribute diagnostics

2018-02-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Thanks! I'll give @aaron.ballman a chance to take a look, so I'll probably do something about this tomorrow. https://reviews.llvm.org/D43359 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D43359: Clean up 'target' attribute diagnostics

2018-02-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: test/Sema/attr-target-ast.c:1 +// RUN: %clang_cc1 -triple x86_64-linux-gnu -ast-dump %s | FileCheck %s + aaron.ballman wrote: > Can you drop the svn props from this file? Absolutely, I'll do that on commit.

[PATCH] D43359: Clean up 'target' attribute diagnostics

2018-02-16 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 134643. erichkeane added a comment. Do as @aaron.ballman suggested for the message. Also, removed properties from the new file. https://reviews.llvm.org/D43359 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclAttr.cpp test/Sema/

[PATCH] D43359: Clean up 'target' attribute diagnostics

2018-02-16 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC325364: Clean up 'target' attribute diagnostics (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D43359?vs=134643&id=134648#toc Repository: rC Clang https:/

[PATCH] D43628: [Sema][NFC] Split Function MultiVersioning decl semantic analysis into its own .cpp file.

2018-02-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: rnk, echristo, aaron.ballman, rsmith. Herald added subscribers: mgrang, mgorny. I'm currently working on the cpu_dispatch and cpu_specific multiversion support, which is making this functionality require a significant amount of function

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

2018-02-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. It seems to me that the test here is very much lacking. It doesn't seem to include the example you've mentioned, and has no validation to ensure that there is a single instantiation happening. I'd like to see what happens when a UUID is passed as a pack, how SFINAE

[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning

2018-02-26 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: include/clang/CodeGen/CGFunctionInfo.h:519 + /// Whether this function has nocf_check attribute. + unsigned NoCfCheck : 1; + aaron.ballman wrote: > oren_ben_simhon wrote: > > aaron.ballman wrote: > > > This is unfor

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

2017-06-13 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Anyone have a chance to look at this? I'll note that this is in no way MSVC specific, this is a code-health issue. https://reviews.llvm.org/D32046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

2017-06-13 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Sure, I'll work on that, thanks! https://reviews.llvm.org/D32046 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

2017-06-13 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 102458. erichkeane added a comment. Added unit test to validate that this still works throughout the range of arguments, and asserts as soon as you get to the end as requested. https://reviews.llvm.org/D32046 Files: include/clang/Lex/MacroArgs.h lib

[PATCH] D32046: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, correct getNumArguments

2017-06-14 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. erichkeane marked 3 inline comments as done. Closed by commit rL305425: [Preprocessor]Correct Macro-Arg allocation of StringifiedArguments, (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D320

[PATCH] D34370: Fix for Bug 33471: Preventing operator auto from resolving to a template operator.

2017-06-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. As the bug report says, struct A { template operator T(); }; void foo() { A().operator auto(); } causes: "undeduced type in IR-generation UNREACHABLE executed at llvm/tools/clang/lib/CodeGen/CodeGenFunction.cpp:208!" The problem is that in this case, "T

[PATCH] D34370: Fix for Bug 33471: Preventing operator auto from resolving to a template operator.

2017-06-20 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 103224. erichkeane marked an inline comment as done. erichkeane added a comment. Hi @rsmith Thanks for the quick response! I spent a while going over it, and think I have what you were looking for. I Also added the operator auto * tests. https://revie

[PATCH] D34370: Fix for Bug 33471: Preventing operator auto from resolving to a template operator.

2017-06-20 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: lib/Sema/SemaLookup.cpp:870 + DeclarationName::CXXConversionFunctionName && + R.getLookupName().getCXXNameType()->getContainedDeducedType() && + R.getLookupName() rsmith wrote: > Maybe only call thi

[PATCH] D34370: Fix for Bug 33471: Preventing operator auto from resolving to a template operator.

2017-06-20 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305812: Fix for Bug 33471: Preventing operator auto from resolving to a template… (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34370?vs=103224&id=103235#toc Repository:

[PATCH] D34455: Correct VectorCall x86 (32 bit) behavior for SSE Register Assignment

2017-06-21 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. In running some internal vectorcall tests in 32 bit mode, we discovered that the behavior I'd previously implemented for x64 (and applied to x32) regarding the assignment of SSE registers was incorrect. See spec here: https://msdn.microsoft.com/en-us/library/d

[PATCH] D34455: Correct VectorCall x86 (32 bit) behavior for SSE Register Assignment

2017-06-21 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305928: Correct VectorCall x86 (32 bit) behavior for SSE Register Assignment (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34455?vs=103391&id=103412#toc Repository: rL

[PATCH] D34455: Correct VectorCall x86 (32 bit) behavior for SSE Register Assignment

2017-06-21 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Thanks Reid! Repository: rL LLVM https://reviews.llvm.org/D34455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier

2017-06-23 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL306149: Emit warning when throw exception in destruct or dealloc functions which has a (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D3?vs=103519&id=103765#toc Reposi

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2017-12-14 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9333 +def err_multiversion_duplicate : Error< + "multiversion function duplicate declarations require identical target " + "attributes">; aaron.ballman wrote: > instead of

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2017-12-14 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 127005. erichkeane added a comment. Fix error message per @aaron.ballman s suggestion. https://reviews.llvm.org/D40819 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/Targ

[PATCH] D41303: Add support for ObjectFormat to TargetSpecificAttr

2017-12-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: aaron.ballman, echristo. Looking through the code, I saw a FIXME on IFunc to switch it to a target specific attribute. In looking through it, i saw that the no-longer-appropriately-named TargetArch didn't support ObjectFormat checking.

[PATCH] D41303: Add support for ObjectFormat to TargetSpecificAttr

2017-12-20 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC321201: Add support for ObjectFormat to TargetSpecificAttr (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D41303?vs=127159&id=127757#toc Repository: rC Cla

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2017-12-21 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. I'm sure most of you have taken off for the year, but if anyone had time, *ping* :D https://reviews.llvm.org/D40819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Hi all-- I'm intending to miss the branch for 6.0, but I'd love to get this in soon after. Can anyone take another look? Thanks, Erich https://reviews.llvm.org/D40819 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D41736: make attribute instantiation instantiate all attributes

2018-01-04 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. Presumably this didn't break any tests, so I'm Ok with it. The initial patch was to make sure that template specializations could clear certain attributes, so I think this properly gets that done. The below test was apparently missed in the initial patch (and was t

[PATCH] D41736: make attribute instantiation instantiate all attributes

2018-01-04 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. LGTM, I like the shouldInheritEvenIfAlreadyPresent name better than isInherited... FWIW. I'll let @aaron.ballman make the final call though. Repository: rC Clang https://reviews.llvm.org/D41736 ___ cfe-commits mailin

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-05 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 34 inline comments as done. erichkeane added a comment. Patch incoming, sorry it took so long! Comment at: lib/CodeGen/CGBuiltin.cpp:7673 -Value *CodeGenFunction::EmitX86CpuInit() { +Value *CodeGenFunction::EmitX86CpuInit(CGBuilderTy &Builder) { llvm::Fun

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-05 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 128782. erichkeane marked 7 inline comments as done. erichkeane added a comment. Fixes for all @echristo and @rsmith s comments. https://reviews.llvm.org/D40819 Files: include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/Basic/Attr

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-05 Thread Erich Keane via Phabricator via cfe-commits
erichkeane planned changes to this revision. erichkeane added a comment. Forgot friend functions, Working on it now, sorry about that. https://reviews.llvm.org/D40819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-05 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 128792. erichkeane added a comment. This revision is now accepted and ready to land. Added test for friend functions. https://reviews.llvm.org/D40819 Files: include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/Basic/Attr.td inclu

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-05 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 128793. erichkeane added a comment. Sorry for the thrash, thought of a better way to do the test just as I clicked 'save' last time :) https://reviews.llvm.org/D40819 Files: include/clang/AST/ASTContext.h include/clang/AST/Decl.h include/clang/Bas

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane marked 7 inline comments as done. erichkeane added a comment. patch incoming! Comment at: include/clang/AST/ASTContext.h:2643-2648 +for (auto *CurDecl : + FD->getDeclContext()->getRedeclContext()->lookup(FD->getDeclName())) { + FunctionDecl *CurFD =

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 128950. erichkeane marked 2 inline comments as done. erichkeane added a comment. All of @rsmith 's comments on the last patch. 1 "open" item on the duplicate discover in the resolver generation, but hopefully this otherwise acceptable. https://reviews.l

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane closed this revision. erichkeane marked 2 inline comments as done. erichkeane added a comment. Closed with revision 322028 https://reviews.llvm.org/D40819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D41837: Add Function multiversion to the release notes.

2018-01-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: rsmith, hans, aaron.ballman, echristo. Richard suggested that I add this feature to the release notes, so I was hoping someone (anyone willing:) ) could do a quick read through for me. Thanks! -Erich Repository: rC Clang https://r

[PATCH] D41837: Add Function multiversion to the release notes.

2018-01-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane updated this revision to Diff 128982. erichkeane added a comment. Ah, right :) Slipped my mind that ELF != x86-linux. https://reviews.llvm.org/D41837 Files: docs/ReleaseNotes.rst Index: docs/ReleaseNotes.rst === ---

[PATCH] D41837: Add Function multiversion to the release notes.

2018-01-08 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC322043: Document attribute target multiversioning. (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D41837?vs=128982&id=128995#toc Repository: rC Clang http

[PATCH] D41952: Clang test changes for 'no-overflow' flag for sdiv\udiv instructions

2018-01-11 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: magabari, zvi, craig.topper, DavidKreitzer, hsaito, nlopes, reames, MatzeB, eli.friedman, rengolin, hfinkel. Herald added subscribers: kbarton, nemanjai. See: https://reviews.llvm.org/D41944 These are the Clang CodeGen test changes req

[PATCH] D44032: Remove -i command line option, add -imultilib

2018-03-02 Thread Erich Keane via Phabricator via cfe-commits
erichkeane created this revision. erichkeane added reviewers: rsmith, chandlerc, aaron.ballman, echristo. I discovered that '-i' is a command line option for the driver, however it actually does not do anything and is not supported by any other compiler. In fact, it is completely undocumented fo

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