[PATCH] D37308: Interface class with uuid base record

erichkeane requested changes to this revision. erichkeane added a comment. This revision now requires changes to proceed. When forming 'diffs', please make sure you do so against the current status of Clang. Your latest diff seems to be only against your first commit, so the diff is crazy looki

[PATCH] D37312: Add documentation for force_align_arg_pointer function attribute

erichkeane added a comment. In https://reviews.llvm.org/D37312#858108, @aaron.ballman wrote: > LGTM! Thank you for working on this! Did you want to land this, or should I? https://reviews.llvm.org/D37312 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D37312: Add documentation for force_align_arg_pointer function attribute

This revision was automatically updated to reflect the committed changes. Closed by commit rL312281: Add documentation for force_align_arg_pointer function attribute (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D37312?vs=113458&id=113469#toc Repository: rL LLVM

[PATCH] D37312: Add documentation for force_align_arg_pointer function attribute

erichkeane added a comment. In https://reviews.llvm.org/D37312#858217, @anatol.pomozov wrote: > Thank you folks for your work. > > By the way when these changes are going to be released? clang 6.0? Most likely. Its too late to get into 5.0, and 6.0 hasn't been branched yet. Repository: rL

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. In https://reviews.llvm.org/D37308#858309, @zahiraam wrote: > Aaron, > > Yes I want to this to succeed: > > struct __declspec(uuid("---C000-0046")) IUnknown {}; > __interface ISfFileIOPropertyPage : public IUnknown {}; > > But I also want this

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

erichkeane added a comment. I apologize for the lack of detail for this comment, but I didn't seem to capture this at the time. When I investigated this initially, I came up with this solution and it was insufficient. I don't remember WHAT this ends up not fixing however. When discussing it

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

erichkeane added a comment. In https://reviews.llvm.org/D37448#861069, @RKSimon wrote: > In https://reviews.llvm.org/D37448#861019, @erichkeane wrote: > > > I apologize for the lack of detail for this comment, but I didn't seem to > > capture this at the time. When I investigated this initially

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

erichkeane added a comment. Ah, I seem to remember that the problem here is that we don't know whether this value should be signed or unsigned at this point, so it could be CreateZExtOrTrunc OR CreateSignExtOrTrunc? However, there is no information as to whether the integer is signed here as f

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

erichkeane added a comment. In https://reviews.llvm.org/D37448#861211, @rnk wrote: > I think this is a reasonable stop-gap fix since the code isn't trying to > return EAX:EDX or XMM0 from the inline asm blob. This affects any function > that contains inline asm and returns a vector, which is po

[PATCH] D37448: Fix cast assertion on MS inline assembly with vector spills (PR34021)

erichkeane added a comment. In https://reviews.llvm.org/D37448#861443, @efriedma wrote: > > According to our definition, v4si is NOT a "Vector" type, since a vector > > type requires it be a FP value. > > Umm, what? An integer vector is still a vector. The backend will return it > in xmm0 on

[PATCH] D37308: Interface class with uuid base record

erichkeane added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:2377 +/// \brief Tests if the __interface base is public. +static bool IsBasePublicInterface(const CXXRecordDecl *RD, + AccessSpecifier spec) { This function i

[PATCH] D37308: Interface class with uuid base record

erichkeane requested changes to this revision. erichkeane added inline comments. This revision now requires changes to proceed. Comment at: lib/Sema/SemaDeclCXX.cpp:2377 +/// \brief Tests if the __interface base is public. +static bool IsRecordPublicInterface(const CXXRecordDecl

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. 1 more thing I missed. Comment at: lib/Sema/SemaDeclCXX.cpp:2389 + return RD->isStruct() && RD->getName() == "IUnknown" && RD->isEmpty() && + Uuid && Uuid->getGuid() =="---C000-0046"; +} This also has

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. By my reading of the code, you still haven't fixed this example: struct __declspec(uuid("---C000-0046")) IUnknown {}; struct IPropertyPageBase : public IUnknown {}; struct IPropertyPage : public IPropertyPageBase {}; __interface foo {

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. You seem to have had a hard time with the diff tool too... there is an extra file here that needs to be removed. Comment at: lib/Sema/SemaDeclCXX.cpp:2390 + Uuid && Uuid->getGuid() =="---C000-0046" && + dyn_ca

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. Also, Craig mentioned to me that naming rules require static fucntions to start with a lower case letter (not uppercase). Additionally, Variables (like 'result') need to start with a capital letter. Comment at: lib/Sema/SemaDeclCXX.cpp:2388 + + re

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. Tighten up the 'IUnknown' check, and do the check I mentioned above, and I think this logic is correct. Searching would be required in the positive case, but this is the negative case. Comment at: lib/Sema/SemaDeclCXX.cpp:2483 +!IsDecl

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. Actually... disregard that... the rule is more complex than that. Based on some playing around with MSVC on godbolt, it seems that it actually marks any type that inherits ONLY from interface types or IUnknown as an interface itself. We may be better off doing that

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. Yeah, but __interface IF1 {}; __interface PP : IUnknown, IF1{}; __interface PP2 : PP, Page3, Page4{}; Base PP has siblings here. It seems the rule is even more complex then. https://reviews.llvm.org/D37308 ___ c

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. A bit more research based on a different implementation: First, there are TWO special types, not just IUnknown! There is also "IDispatch" with UUID: "00020400---c000-0046" A type is 'interface like' if: -it has a ZERO virtual inheritance bases -it ha

[PATCH] D37308: Interface class with uuid base record

erichkeane added a comment. SO, the implementation would likely be (~2489): if (Class->isInterface() && (KnownBase->getAccessSpecifier() != TTK_public || !RD->isInterface() || !RD->isInterfaceLike()) { with "isInterfaceLike" testing what I outlined above. https://reviews.llvm.org/D37308

[PATCH] D37308: Interface class with uuid base record

erichkeane commandeered this revision. erichkeane edited reviewers, added: zahiraam; removed: erichkeane. erichkeane added a comment. @zahiraam: quickly commendeering, since I have an implementation that I think better explains my discovery than my words. Feel free to commandeer it back later (

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

erichkeane updated this revision to Diff 115153. erichkeane retitled this revision from "Interface class with uuid base record" to "Fix the __interface inheritence rules to work better with IUnknown and IDispatch". erichkeane edited the summary of this revision. erichkeane added a comment. Herald

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

erichkeane marked 4 inline comments as done. erichkeane added a comment. Thanks for the quick response! New patch coming soon. Comment at: lib/AST/DeclCXX.cpp:1478 + + // No interface-like types can have a user declared constructor, destructor, + // friends, VBases, conersi

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

erichkeane updated this revision to Diff 115272. erichkeane marked an inline comment as done. erichkeane added a comment. Fixed for @rnk s comments. https://reviews.llvm.org/D37308 Files: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/Sema/SemaDeclCXX.cpp test/SemaCXX/ms-iunknown-i

[PATCH] D37865: [Sema] Fix a pair of crashes when generating exception specifiers with an error'ed field for a template class' default ctor.

erichkeane created this revision. The two examples in the test would both cause a compiler assert when attempting to calculate the exception specifier for the default constructor for the template classes. The problem was that dependents of this function expect that Field->getInClassInitializer

[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

This revision was automatically updated to reflect the committed changes. Closed by commit rL313364: Fix the __interface inheritence rules to work better with IUnknown and IDispatch (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D37308?vs=115272&id=115414#toc Repos

[PATCH] D32052: [Preprocessor] Dont omit comma after __VA_ARGS__ in MSVC mode if it is an argument to a macro function that takes only a single parameter.

erichkeane abandoned this revision. erichkeane added a comment. This has likely gone stale, and my employer has decided to tell customers to modify their code. https://reviews.llvm.org/D32052 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32049: [Preprocessor] Implement empty vs omitted __VA_ARGS__ string-expansion MSVC Extension

erichkeane abandoned this revision. erichkeane added a comment. This has likely gone stale, and my employer has decided to tell customers to modify their code. https://reviews.llvm.org/D32049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32050: [Preprocessor] Implement MSVC extension to expand arguments to ##

erichkeane abandoned this revision. erichkeane added a comment. This has likely gone stale, and my employer has decided to tell customers to modify their code. https://reviews.llvm.org/D32050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D32049: [Preprocessor] Implement empty vs omitted __VA_ARGS__ string-expansion MSVC Extension

erichkeane added a comment. I don't know? I don't have a really good way of repro'ing that. Do you have any ability to grab me a preprocessed reproducer of that one? Godbolt doesn't have Windows headers for this. https://reviews.llvm.org/D32049 ___

[PATCH] D32051: [Preprocessor] Improve MSVC comma-elision before an empty expansion.

erichkeane abandoned this revision. erichkeane added a comment. This has likely gone stale, and my employer has decided to tell customers to modify their code. https://reviews.llvm.org/D32051 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37865: [Sema] Fix a pair of crashes when generating exception specifiers with an error'ed field for a template class' default ctor.

erichkeane added a comment. Ping? I Added the three of you because you were all on the git-blame list, but if you have better ideas on someone to review this, it would be appreciated. -Erich https://reviews.llvm.org/D37865 ___ cfe-commits mailing

[PATCH] D37865: [Sema] Fix a pair of crashes when generating exception specifiers with an error'ed field for a template class' default ctor.

This revision was automatically updated to reflect the committed changes. Closed by commit rL313569: [Sema] Fix a pair of crashes when generating exception specifiers with an (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D37865?vs=115278&id=115726#toc Repository:

[PATCH] D38092: [MS Compat]Allow __interfaces to have properties.

erichkeane created this revision. __interface types are allowed in MSVC to have "property" data members (marked with declspec property). This patch alters Sema to allow property data members. https://reviews.llvm.org/D38092 Files: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/ms-interface.cpp In

[PATCH] D38092: [MS Compat]Allow __interfaces to have properties.

erichkeane added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:2871 + InvalidDecl = + (DS.getStorageClassSpec() == DeclSpec::SCS_typedef || MSPropertyAttr) + ? 0 Note: Clang format did this craziness... I'm open to whatever form

[PATCH] D38092: [MS Compat]Allow __interfaces to have properties.

This revision was automatically updated to reflect the committed changes. Closed by commit rL313828: [MS Compat]Allow __interfaces to have properties. (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D38092?vs=116046&id=116097#toc Repository: rL LLVM https://revie

[PATCH] D38145: Suppress Wsign-conversion for enums with matching underlying type

erichkeane created this revision. As reported here: https://bugs.llvm.org/show_bug.cgi?id=34692 A non-defined enum with a backing type was always defaulting to being treated as a signed type. IN the case where it IS defined, the signed-ness of the actual items is used. This patch uses the unde

[PATCH] D38145: Suppress Wsign-conversion for enums with matching underlying type

erichkeane updated this revision to Diff 116244. erichkeane marked 2 inline comments as done. erichkeane added a comment. Updated based on Eli's feedback. https://reviews.llvm.org/D38145 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-sign-conversion-cpp11.cpp Index: test/SemaCXX/warn-

[PATCH] D38145: Suppress Wsign-conversion for enums with matching underlying type

erichkeane added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8176 + // underlying type, so that when someone specifies the type as + // "unsigned" it doesn't cause sign-conversion type warnings. if (!Enum->isCompleteDefinition()) efrie

[PATCH] D38145: Suppress Wsign-conversion for enums with matching underlying type

erichkeane updated this revision to Diff 116245. erichkeane marked an inline comment as done. https://reviews.llvm.org/D38145 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/warn-sign-conversion-cpp11.cpp Index: test/SemaCXX/warn-sign-conversion-cpp11.cpp =

[PATCH] D38145: Suppress Wsign-conversion for enums with matching underlying type

This revision was automatically updated to reflect the committed changes. Closed by commit rL313907: Suppress Wsign-conversion for enums with matching underlying type (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D38145?vs=116245&id=116252#toc Repository: rL LLV

[PATCH] D38202: Add Documentation to attribute-nothrow. Additionally, limit to functions.

erichkeane created this revision. Herald added a subscriber: javed.absar. Attribute nothrow is only allowed on functions, so I added that. Additionally, it lacks any documentation, so I added some. Please wordsmith! https://reviews.llvm.org/D38202 Files: include/clang/Basic/Attr.td includ

[PATCH] D38203: [Sema] Corrected the warn-on-throw-from-noexcept behavior to include nothrow

erichkeane created this revision. Discovered that 'nothrow' (which is supposed to be an alias for noexcept) was not warning with a throw inside of it. This patch corrects the behavior previously created to add 'nothrow' to this list. https://reviews.llvm.org/D38203 Files: lib/Sema/AnalysisBa

[PATCH] D38205: [Sema] Warn on attribute nothrow conflicting with language specifiers

erichkeane created this revision. I discovered it was possible to create a 'nothrow' noexcept(false) function, which is both non-sensical as well as seemingly breaking. This patch warns if attribute nothrow is used with anything besides "noexcept". "noexcept(true)" isn't possible, because the no

[PATCH] D38205: [Sema] Warn on attribute nothrow conflicting with language specifiers

erichkeane updated this revision to Diff 116433. erichkeane added a comment. Woops, didn't commit my correct tests! https://reviews.llvm.org/D38205 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclAttr.cpp test/SemaCXX/warn-conflicting-nothrow-attr-exception-spec.cpp I

[PATCH] D38209: [Sema] Correct nothrow inherited by noexcept

erichkeane created this revision. As reported in https://bugs.llvm.org/show_bug.cgi?id=33235, a noexcept function was unable to inherit from a nothrow defaulted constructor. Attribute "nothrow" is supposed to be semantically identical to noexcept, and in fact, a number of other places in the code

[PATCH] D62665: Fix constexpr __builtin_*_overflow issue when unsigned->signed operand.

erichkeane created this revision. erichkeane added reviewers: eli.friedman, efriedma, craig.topper. As reported here https://bugs.llvm.org/show_bug.cgi?id=42000, it was possible to get the constexpr version of __builtin_*_overflow to give the wrong answer. This was because when extending the oper

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane marked 6 inline comments as done. erichkeane added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1547 + bool hasAttr(ParsedAttr::Kind Kind) const { +return llvm::find_if(getAttrs(), [Kind](const ParsedAttr &P) { aaron.ballman

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

This revision was automatically updated to reflect the committed changes. erichkeane marked 2 inline comments as done. Closed by commit rL362119: Add Attribute NoThrow as an Exception Specifier Type (authored by erichkeane, committed by ). Herald added a project: LLVM. Herald added a subscriber: l

[PATCH] D62665: Fix constexpr __builtin_*_overflow issue when unsigned->signed operand.

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9457 + LHS = APSInt(LHS.isSigned() ? LHS.sextOrSelf(MaxBits) + : LHS.zextOrSelf(MaxBits), !IsSigned)

[PATCH] D62665: Fix constexpr __builtin_*_overflow issue when unsigned->signed operand.

erichkeane updated this revision to Diff 202268. erichkeane marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62665/new/ https://reviews.llvm.org/D62665 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/builtins-overflow.cpp Index: clang/test/Se

[PATCH] D62665: Fix constexpr __builtin_*_overflow issue when unsigned->signed operand.

This revision was automatically updated to reflect the committed changes. Closed by commit rL362157: Fix constexpr __builtin_*_overflow issue when unsigned->signed operand. (authored by erichkeane, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane added a comment. In D62435#1524824 , @mstorsjo wrote: > This change broke compiling Qt on MinGW, see > https://bugs.llvm.org/show_bug.cgi?id=42089. Trivially reproducible by trying > to compile a snippet that looks like this: > > class Foo {

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

erichkeane added a comment. In D56571#1523895 , @nickdesaulniers wrote: > Was this committed accidentally? > > Today in master I see: > > - r362106: Revert "clang support gnu asm goto." Erich Keane > > - r362059: Mark CodeGen/asm-goto.c as x86 specific

[PATCH] D63283: PR42182: Allow thread-local to use __cxa_thread_atexit when -fno-use-cxx-atexit is used

erichkeane created this revision. erichkeane added reviewers: jyu2, rnk, ahatanak, rjmccall, jyknight. Herald added a subscriber: dexonsmith. Herald added a project: clang. This matches the GCC behavior, __cxa_thread_atexit should be permissible even though cxa_atexit is disabled. Repository:

[PATCH] D63283: PR42182: Allow thread-local to use __cxa_thread_atexit when -fno-use-cxx-atexit is used

erichkeane updated this revision to Diff 204587. erichkeane marked an inline comment as done. erichkeane added a comment. Update the comment as requested by @rjmccall CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63283/new/ https://reviews.llvm.org/D63283 Files: clang/lib/CodeGen/Ita

[PATCH] D64914: Implement P1771

erichkeane created this revision. erichkeane added reviewers: aaron.ballman, chandlerc. Herald added a project: clang. EWG met on 7/18/19 and came to the conclusion that our initial intent was to apply to constructors, so approved the paper and expected implementers to use implementers discretion

[PATCH] D64914: Implement P1771

erichkeane marked 5 inline comments as done. erichkeane added a comment. In D64914#1591744 , @aaron.ballman wrote: > There seems to be some separable concerns in this patch. I'd rather real with > `warn_unused_result`, `const`, and `pure` attributes sepa

[PATCH] D64914: Implement P1771

erichkeane updated this revision to Diff 210753. erichkeane marked an inline comment as done. erichkeane added a comment. Removes pure/const, limits change to just CXX11 spelling, and changes docs/FeatureTestMacro. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://review

[PATCH] D64914: Implement P1771

erichkeane marked 2 inline comments as done. erichkeane added a comment. I'll need to rebase this on another patch soon anyway, so I'll hold off until next week to update this particularly since we have some open questions. The additional TableGen work is tempting to do, though I'm not complete

[PATCH] D64914: Implement P1771

erichkeane updated this revision to Diff 211094. erichkeane marked an inline comment as done. erichkeane added a comment. Rebased and did all the comments (including the www_status). @aaron.ballman : Wasn't positive what you meant about the conversion functions, but I think I got one? I would

[PATCH] D64914: Implement P1771

erichkeane marked 2 inline comments as done. erichkeane added a comment. Ugg... well conversion functions are an interesting bit, as well as Type{}; with no constructors. It ends up being a function style cast to an init-list-expr, so its going to require a bit more work. Stay tuned :)

[PATCH] D64914: Implement P1771

erichkeane updated this revision to Diff 211482. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.org/D64914 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/AST/

[PATCH] D64914: Implement P1771

erichkeane added a comment. Disregard that last commit... i ended up breaking stuff apparently. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.org/D64914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D64914: Implement P1771

erichkeane updated this revision to Diff 211504. erichkeane added a comment. Casting implementation had to happen in SemaStmt, because other 'warn unused result' stuff depends on them being the way they are. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.

[PATCH] D64914: Implement P1771

erichkeane updated this revision to Diff 211740. erichkeane marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64914/new/ https://reviews.llvm.org/D64914 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/

[PATCH] D64914: Implement P1771

This revision was automatically updated to reflect the committed changes. Closed by commit rL367027: Implement P1771 (authored by erichkeane, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D64914?vs=211740

[PATCH] D65294: Make the CXXABIs respect the target's default calling convention.

erichkeane created this revision. erichkeane added a reviewer: Anastasia. SPIR targets need to have all functions be SPIR calling convention, however the CXXABIs were just returning CC_C in all non-'this-CC' cases. https://reviews.llvm.org/D65294 Files: clang/lib/AST/ItaniumCXXABI.cpp clang

[PATCH] D65294: Make the CXXABIs respect the target's default calling convention.

erichkeane added a comment. Committed, 9.0 release request here: https://bugs.llvm.org/show_bug.cgi?id=42774 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65294/new/ https://reviews.llvm.org/D65294 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D64015: [WIP][CUDA] Use shared MangleContext for CUDA and CXX CG

erichkeane added a comment. I don't know of any problems that the shared mangle context would cause, though I'm not sure about using the shared_ptr. It seems to me that SOMEONE should own this, and the other should use a reference. IMO, the SharedMangleContexts should be a unique_ptr and CGCXX

[PATCH] D64276: [ItaniumMangle] Refactor long double/__float128 mangling and fix the mangled code

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I think the function names are awkward, but otherwise I think this should be acceptable. I'll accept, but give 24 hrs for the other reviewers to get through their mondays.

[PATCH] D63846: [clang] Preserve names of addrspacecast'ed values.

erichkeane added a comment. The change itself doesn't really bother me at all, however the test will fail if we build without preserving names. In order to validate this you'll have to do fno-discard-value-names on a test. Comment at: clang/test/CodeGenOpenCL/address-spaces-

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

erichkeane added inline comments. Comment at: include/llvm/IR/IRBuilder.h:110 : Context(context), DefaultFPMathTag(FPMathTag), -DefaultOperandBundles(OpBundles) { +IsFPConstrained(false), DefaultConstrainedExcept(nullptr), +DefaultConstrainedRoundin

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane created this revision. erichkeane added reviewers: rnk, lebedev.ri, aaron.ballman. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. In response to https://bugs.llvm.org/show_bug.cgi?id=33235, it became clear that the current mechanism of hacking through ch

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane marked 2 inline comments as done. erichkeane added inline comments. Comment at: clang/include/clang-c/Index.h:202-204 + * The cursor has a declspec(nothrow) exception specification. + */ + CXCursor_ExceptionSpecificationKind_NoThrow, rsmith wrote:

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane updated this revision to Diff 201404. erichkeane marked 2 inline comments as done. erichkeane added a comment. Reread @rsmith's comments and surrounding code and found what I believe is the correct answer to these comments :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D6243

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:6970 +if (Proto->hasExceptionSpec()) + return true; + aaron.ballman wrote: > I think we should diagnose that the `nothrow` is ignored i

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

erichkeane updated this revision to Diff 201673. erichkeane marked 5 inline comments as done. erichkeane added a comment. Added warning + other comments from @aaron.ballman The exception state was further along than I expected, so I was able to make the warning better than I thought! CHANGES S

[PATCH] D62580: [OpenCL] Use long instead of long long in x86 builtins

erichkeane added a comment. 'O' is an interesting choice. Any real justification for it, or just "what was available"? It definitely needs to be documented in the top of Builtins.def however. Comment at: clang/lib/AST/ASTContext.cpp:9362 +case 'O': + assert(!IsSpec

[PATCH] D62580: [OpenCL] Use long instead of long long in x86 builtins

erichkeane added a comment. A different (perhaps silly) question is why 'W' isn't sufficient? It represents int64_t, which I wonder if is sufficient. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62580/new/ https://reviews.llvm.org/D62580

[PATCH] D62580: [OpenCL] Use long instead of long long in x86 builtins

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Conditional approval, commit with the comment fixed. Let me know if you need me to commit it for you. Comment at: clang/include/clang/Basic/Builtins.def:56 // N

[PATCH] D55527: Normalize GlobalDecls when used with CPUDispatch

erichkeane marked 7 inline comments as done. erichkeane added a comment. I got a couple of @rsmith's requests done. Most importantly I suspect is `MultiVersionFuncs`, though `NotForDefinition` vs `ForDefinition` is perhpas something you'll find important. Comment at: lib/Cod

[PATCH] D55527: Normalize GlobalDecls when used with CPUDispatch

erichkeane updated this revision to Diff 177853. erichkeane added a comment. As mentioned, the @rsmith comments that I thought were doable without feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55527/new/ https://reviews.llvm.org/D55527 Files: include/clang/AST/GlobalDecl.h

[PATCH] D53713: Add extension to always default-initialize nullptr_t.

erichkeane added a comment. Is there any feedback here? Am I just completely incorrect in how I tried to fix this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53713/new/ https://reviews.llvm.org/D53713 ___ cfe-commits mailing list cfe-c

[PATCH] D55715: Add AddressSpace mangling to MS mode

erichkeane created this revision. erichkeane added reviewers: zturner, majnemer, compnerd, rnk. Herald added subscribers: erik.pilkington, Anastasia. All of the symbols demangle on llvm-undname and demangler.com. This address space qualifier is useful for when we want to use opencl C++ in Windows

[PATCH] D55715: Add AddressSpace mangling to MS mode

erichkeane marked 4 inline comments as done. erichkeane added inline comments. Comment at: test/CodeGenCXX/mangle-address-space.cpp:7 +// CHECKNOOCL-LABEL: define {{.*}}void @_Z2f0Pc +// WINNOOCL-LABEL: define {{.*}}void @"?f0@@YAXPEAD@Z" +// CHECKOCL-LABEL: define {{.*}}void @_Z

[PATCH] D55715: Add AddressSpace mangling to MS mode

erichkeane marked 2 inline comments as done. erichkeane added inline comments. Comment at: test/CodeGenOpenCL/address-spaces-mangling.cl:7 +// RUN: %clang_cc1 %s -ffake-address-space-map -faddress-space-map-mangling=yes -triple x86_64-windows-pc -emit-llvm -o - | FileCheck -che

[PATCH] D55715: Add AddressSpace mangling to MS mode

erichkeane marked 2 inline comments as done. erichkeane added inline comments. Comment at: lib/AST/MicrosoftMangle.cpp:1806-1836 +Extra.mangleSourceName("AS_"); +Extra.mangleIntegerLiteral(llvm::APSInt::getUnsigned(TargetAS), + /*IsBoolean*/

[PATCH] D55715: Add AddressSpace mangling to MS mode

erichkeane updated this revision to Diff 178282. erichkeane added a comment. Should catch me up on all comments except @zturner's llvm-undname feature request :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55715/new/ https://reviews.llvm.org/D55715 Files: lib/AST/MicrosoftMangle.c

[PATCH] D53713: Add extension to always default-initialize nullptr_t.

This revision was automatically updated to reflect the committed changes. Closed by commit rC349201: Add extension to always default-initialize nullptr_t. (authored by erichkeane, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53713/new/ https://rev

[PATCH] D53713: Add extension to always default-initialize nullptr_t.

erichkeane added a comment. As stated in CFE commits (in response to Richard's comments): `I’ll push a revert in a few minutes and give it another try. Perhaps I can suppress the creation of a CK_LValueToRValue in the case where it’s a read from nullptr_t Repository: rC Clang CHANGES SINC

[PATCH] D55715: Add AddressSpace mangling to MS mode

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: test/CodeGenCXX/mangle-address-space.cpp:7 +// CHECKNOOCL-LABEL: define {{.*}}void @_Z2f0Pc +// WINNOOCL-LABEL: define {{.*}}void @"?f0@@YAXPEAD@Z" +// CHECKOCL-LABEL: define {{.*}}void @_Z

[PATCH] D55715: Add AddressSpace mangling to MS mode

This revision was automatically updated to reflect the committed changes. Closed by commit rL349209: Add AddressSpace mangling to MS mode (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55715?vs=178282&id=17830

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

erichkeane created this revision. erichkeane added reviewers: majnemer, rnk, pengfei. As reported in PR33035, LLVM crashes if given a common object with an alignment of greater than 32 bits. This is because the COFF file format does not support these alignments, so emitting them is broken anyway.

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

erichkeane updated this revision to Diff 180650. erichkeane marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56391/new/ https://reviews.llvm.org/D56391 Files: lib/CodeGen/CodeGenModule.cpp test/CodeGen/microsoft-no-common-align.c Index: test/CodeGen/m

[PATCH] D56407: Implement the TreeStructure interface through the TextNodeDumper

erichkeane added a comment. In D56407#1349672 , @aaron.ballman wrote: > I think this is a good approach to solving the problem, so let's go this > route. Yeah, I agree here. This ends up being a really clean solution. Repository: rC Clang CHANGES

[PATCH] D56391: Limit COFF 'common' emission to <=32 alignment types.

This revision was automatically updated to reflect the committed changes. Closed by commit rC350643: Limit COFF 'common' emission to <=32 alignment types. (authored by erichkeane, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56391/new/ https://rev

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc & Fix resolver linkage type

erichkeane added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3002 false); llvm::Constant *Resolver = GetOrCreateLLVMFunction( MangledName + ".resolver", ResolverType, GlobalDecl{}, zsrkmyn wrote: > erichkeane wrote: > >

[PATCH] D67058: [clang][CodeGen] Add alias for cpu_dispatch function with IFunc & Fix resolver linkage type

erichkeane added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3002 false); llvm::Constant *Resolver = GetOrCreateLLVMFunction( MangledName + ".resolver", ResolverType, GlobalDecl{}, zsrkmyn wrote: > erichkeane wrote: > >

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