[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] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2266 let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">, - C2x<"", "maybe_unused">]; + C2x<"", "maybe

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

erichkeane added a comment. In D67368#1669818 , @plotfi wrote: > I think this change might be breaking builds: > http://lab.llvm.org:8011/builders/clang-aarch64-linux-build-cache/builds/16888 Yep! I fixed it in r371876. Repository: rL LLVM CHANGES

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h:166 + ? SpellingIndex + : calculateAttributeSpellingListIndex(); + } aheejin wrote: > MaskR

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h:166 + ? SpellingIndex + : calculateAttributeSpellingListIndex(); + } plotfi wrote: > erichk

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

erichkeane added a comment. Fixed in r371985. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67368/new/ https://reviews.llvm.org/D67368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D67854: Ensure AtomicExpr goes through SEMA checking after TreeTransform

erichkeane created this revision. erichkeane added a reviewer: eli.friedman. Herald added a subscriber: jfb. RebuildAtomicExpr was skipping doing semantic analysis which broke in the cases where the expressions were not dependent. This resulted in the ImplicitCastExpr from an array to a pointer be

[PATCH] D67854: Ensure AtomicExpr goes through SEMA checking after TreeTransform

This revision was automatically updated to reflect the committed changes. Closed by commit rL372422: Ensure AtomicExpr goes through SEMA checking after TreeTransform (authored by erichkeane, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D67854: Ensure AtomicExpr goes through SEMA checking after TreeTransform

erichkeane added a comment. In D67854#1678236 , @RKSimon wrote: > @erichkeane The atomic-expr.cpp test is failing on some MSVC buildbots, > please can you take a look? > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/

[PATCH] D67854: Ensure AtomicExpr goes through SEMA checking after TreeTransform

erichkeane added a comment. In D67854#1678982 , @erichkeane wrote: > In D67854#1678236 , @RKSimon wrote: > > > @erichkeane The atomic-expr.cpp test is failing on some MSVC buildbots, > > please can you take a look?

[PATCH] D67924: [Sema] Fix the atomic expr rebuilding order.

erichkeane added a comment. Yikes, good catch! Would we be better off instead to just modify how the other switch loads the value? Presumably something like, "if (NeedsRearrangeArgs) SubExprs.append(Args.begin(), Args.end()); else /*the switch*/. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D67924: [Sema] Fix the atomic expr rebuilding order.

erichkeane added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:3319 +return getSema().BuildAtomicExpr(Range, Range, RParenLoc, SubExprs, Op, + true); } When passing a bool, use the comment syntax /*paramnam

[PATCH] D67924: [Sema] Fix the atomic expr rebuilding order.

erichkeane added a comment. It kinda stinks that we have to do this at such a late step. I'd have much preferred doing this as a part of the rebuild, but it appears that the 'form' takes quite a bit to calculate. Also, I'd likely have preferred that the initial reordering happened in codegen

[PATCH] D67924: [Sema] Fix the atomic expr rebuilding order.

erichkeane added a comment. Also, can you add a test for GNUCmpXchg in both situations (inside a template, and outside)? It is the most complicated, and would best reflect the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67924/new/ https:/

[PATCH] D67924: [Sema] Fix the atomic expr rebuilding order.

erichkeane added a comment. Seems your test changes have disappeared as well? Otherwise, 1 comment then I'm ok with this. Comment at: clang/lib/Sema/TreeTransform.h:3319 +Range, Range, RParenLoc, SubExprs, Op, +/*AtomicArgumentOrder*/ Sema::AtomicArgumentOrder

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane added a comment. In https://reviews.llvm.org/D45383#1065999, @compnerd wrote: > I know that the Windows SDK definitely declares the `__va_start` function. > Did you try building something like swift against the Windows SDK with this > change? Are we sure it declares it, and not ju

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane added a comment. In https://reviews.llvm.org/D45383#1068085, @compnerd wrote: > Snipping bits from `va_defs.h`: > > > #elif defined _M_ARM64 > > void __cdecl __va_start(va_list*, ...); > > #define __crt_va_start_a(ap,v) ((void)(__va_start(&ap, _ADDRESSOF(v), > _

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane added a comment. In https://reviews.llvm.org/D45383#1069049, @efriedma wrote: > We can could add an exception to the "don't allow redeclarations with custom > type-checking" rule to specifically allow redeclaring `__va_start`. The > general rule is that we don't want user code redec

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane added a comment. In https://reviews.llvm.org/D45383#1069057, @efriedma wrote: > I don't see an `A` in `LANGBUILTIN(__va_start, "vc**.", "nt", > ALL_MS_LANGUAGES)` Ah, you're right! I was confusing it with LIBBUILTIN(va_start, "vA.", "fn", "stdarg.h", ALL_LANGUAGES) and BUILT

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane updated this revision to Diff 142685. erichkeane added a comment. Added the exception for MSVC's __va_start. __va_start is a normal function on Linux, so it isn't an error on non-windows either. https://reviews.llvm.org/D45383 Files: include/clang/AST/ASTContext.h include/clang

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane updated this revision to Diff 142695. erichkeane added a comment. Revert unnecessary test changes... :/ https://reviews.llvm.org/D45383 Files: include/clang/AST/ASTContext.h include/clang/Basic/Builtins.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ASTContext.cpp li

[PATCH] D45383: Limit types of builtins that can be redeclared.

This revision was automatically updated to reflect the committed changes. Closed by commit rL330160: Limit types of builtins that can be redeclared. (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45383?vs=1426

[PATCH] D45738: Add Microsoft mangling for _Float16, similar to technique used for _Complex

erichkeane added a comment. FWIW, this demangles on both demangler.com and undname as "void __cdecl Float16::f(struct __clang::_Float16)", which seems like the right thing, since the "Complex" above demangles to: "void __cdecl Complex::f(struct __clang::_Complex)" I also note (since upon first

[PATCH] D45738: Add Microsoft mangling for _Float16, similar to technique used for _Complex

This revision was automatically updated to reflect the committed changes. Closed by commit rL330225: Add Microsoft mangling for _Float16 (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D45738?vs=142827&id=142846#toc Repository: rL LLVM https://revi

[PATCH] D45812: Fix __attribute__((force_align_arg_pointer)) misalignment bug

erichkeane added a comment. Can you make sure that this doesn't cause stackrealign to happen 2x if using -mstackrealign? Additionally, please provide the diff with full-context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface). What is the commit before th

[PATCH] D45812: Fix __attribute__((force_align_arg_pointer)) misalignment bug

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. LGTM, Do you have commit access, or do you want me to commit it for you? https://reviews.llvm.org/D45812 ___ cfe-commits mailing list cfe

[PATCH] D45812: Fix __attribute__((force_align_arg_pointer)) misalignment bug

This revision was automatically updated to reflect the committed changes. Closed by commit rC330331: Fix __attribute__((force_align_arg_pointer)) misalignment bug (authored by erichkeane, committed by ). Repository: rC Clang https://reviews.llvm.org/D45812 Files: lib/CodeGen/TargetInfo.cpp

[PATCH] D46131: Add Microsoft Mangling for OpenCL Half Type

This revision was automatically updated to reflect the committed changes. Closed by commit rL331263: Add Microsoft Mangling for OpenCL Half Type (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46131?vs=144139&i

[PATCH] D46131: Add Microsoft Mangling for OpenCL Half Type

This revision was automatically updated to reflect the committed changes. Closed by commit rC331263: Add Microsoft Mangling for OpenCL Half Type (authored by erichkeane, committed by ). Repository: rC Clang https://reviews.llvm.org/D46131 Files: lib/AST/MicrosoftMangle.cpp test/CodeGenOpe

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

erichkeane added inline comments. Comment at: include/clang/Frontend/DependencyOutputOptions.h:31 unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency list unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info. + unsigned

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. This looks fine to me. Let me know if you need me to commit this for you. https://reviews.llvm.org/D46394 ___ cfe-commits mailing list cf

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

erichkeane requested changes to this revision. erichkeane added a comment. This revision now requires changes to proceed. When building this, it didn't build since you'd reused the name. I changed the variable to be named "Dest" instead of "Destination" That said, the test suite now fails on Cl

[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P

This revision was automatically updated to reflect the committed changes. Closed by commit rC331533: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP… (authored by erichkeane, committed by ). Repository: rC Clang https://reviews.llvm.org/D46394 Files: include/cl

[PATCH] D46535: Correct warning on Float->Integer conversions.

erichkeane created this revision. erichkeane added reviewers: aaron.ballman, spatel, RKSimon. As identified and briefly discussed here: https://bugs.llvm.org/show_bug.cgi?id=37305 Converting a floating point number to an integer type when the integral part is out of the range of the integer type

[PATCH] D42664: Fix explicit template parameter reporting for narrowing conversions

erichkeane abandoned this revision. erichkeane added a comment. Did Review-after-commit, in retrospect this is a pretty tiny change. https://reviews.llvm.org/D42664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D46535: Correct warning on Float->Integer conversions.

erichkeane marked 2 inline comments as done. erichkeane added inline comments. Comment at: lib/Sema/SemaChecking.cpp:9440-9441 +diag::warn_impcast_literal_float_to_integer_out_of_range); + if (IntegerValue.isUnsigned() && + (IntegerValue.isMaxValue() ||

[PATCH] D46535: Correct warning on Float->Integer conversions.

erichkeane updated this revision to Diff 145536. erichkeane marked an inline comment as done. erichkeane added a comment. Aaron's comments :) https://reviews.llvm.org/D46535 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/SemaCXX/coroutines.cpp test/Sema

[PATCH] D46535: Correct warning on Float->Integer conversions.

This revision was automatically updated to reflect the committed changes. Closed by commit rC331673: Correct warning on Float->Integer conversions. (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D46535?vs=145536&id=145542#toc Repository: rC Clang

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

erichkeane added inline comments. Comment at: test/Sema/attr-target-mv.c:98 int __attribute__((target("sse4.2"))) diff_cc(void); -// expected-error@+1 {{multiversioned function declaration has a different calling convention}} +// expected-error@+1 {{attribute 'target' multivers

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

erichkeane added a comment. In https://reviews.llvm.org/D48100#1142842, @rsmith wrote: > This patch appears to have at least caused us to process attributes too many > times in some cases. For example: > > __attribute__((noreturn,noinline,unused)) void f(); > > > before your patch gives this w

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

This revision was automatically updated to reflect the committed changes. Closed by commit rC336379: [clang-cl, PCH] Implement support for MS-style PCH through headers (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D46652?vs=153067&id=154265#toc Repo

[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers

This revision was automatically updated to reflect the committed changes. Closed by commit rL336379: [clang-cl, PCH] Implement support for MS-style PCH through headers (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane added a comment. Bump! https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49045: PR15730/PR16986 Allow dependently typed vector_size types.

erichkeane created this revision. erichkeane added reviewers: rsmith, aaron.ballman, mibintc, mikerice, Manna. As listed in the above PRs, vector_size doesn't allow dependent types/values. This patch introduces a new DependentVectorType to handle a VectorType that has a dependent size or type.

[PATCH] D45383: Limit types of builtins that can be redeclared.

erichkeane added a comment. I'm curious why that file is declaring the builtin like that? The declaration doesn't seem like it is ever useful, builtins are available without any includes/etc as far back as I could go on all 3 compilers that I validated against. Additionally, I believe that th

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane marked 4 inline comments as done. erichkeane added a comment. In https://reviews.llvm.org/D47474#1154858, @lebedev.ri wrote: > Some drive-by nits. > General observation: this is kinda large. > I would //personally// split split off the codegen part into a second patch. Thanks for t

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane updated this revision to Diff 154610. erichkeane marked an inline comment as done. erichkeane added a comment. @lebedev.ri 's comments, plus a rebase. https://reviews.llvm.org/D47474 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td i

[PATCH] D49045: PR15730/PR16986 Allow dependently typed vector_size types.

erichkeane updated this revision to Diff 155419. erichkeane marked 12 inline comments as done. https://reviews.llvm.org/D49045 Files: include/clang/AST/ASTContext.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Type.h include/clang/AST/TypeLoc.h include/clang/AST/TypeNodes.d

[PATCH] D49045: PR15730/PR16986 Allow dependently typed vector_size types.

erichkeane added inline comments. Comment at: include/clang/AST/ASTContext.h:1327-1329 + QualType getDependentVectorType(QualType VectorType, Expr *SizeExpr, + SourceLocation AttrLoc, + VectorType::Vecto

[PATCH] D46915: [Fixed Point Arithmetic] Fixed Point Precision Bits and Fixed Point Literals

erichkeane added a comment. See this bug here: https://bugs.llvm.org/show_bug.cgi?id=38161 This patch doesn't seem to properly consider integers, and ignores octals, which I suspect shouldn't be the case. Repository: rC Clang https://reviews.llvm.org/D46915 __

[PATCH] D46915: [Fixed Point Arithmetic] Fixed Point Precision Bits and Fixed Point Literals

erichkeane added a comment. In https://reviews.llvm.org/D46915#1162095, @leonardchan wrote: > In https://reviews.llvm.org/D46915#1162038, @erichkeane wrote: > > > See this bug here: https://bugs.llvm.org/show_bug.cgi?id=38161 > > > > This patch doesn't seem to properly consider integers, and igno

[PATCH] D49045: PR15730/PR16986 Allow dependently typed vector_size types.

This revision was automatically updated to reflect the committed changes. Closed by commit rL337036: PR15730/PR16986 Allow dependently typed vector_size types. (authored by erichkeane, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane added a comment. Bump! Would love to have someone take a look! https://reviews.llvm.org/D47474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

erichkeane created this revision. erichkeane added reviewers: majnemer, rjmccall. I discovered that function pointers inside a RecordType that had its type-determination done in a function whose signature matched said function pointer resulted in the function pointer type being emitted empty. Thi

[PATCH] D49403: More aggressively complete RecordTypes with Function Pointers

erichkeane added a subscriber: olga.chupina. erichkeane added a comment. In https://reviews.llvm.org/D49403#1165845, @rjmccall wrote: > Can you explain why this is important for the optimizer? @olga.a.chupina (or is it @olga.chupina ) is the one who reported this to me, so hopefully she can e

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane updated this revision to Diff 156068. erichkeane marked 13 inline comments as done. erichkeane added a comment. All of @aaron.ballman s comments have been dealt with except for Cpus->CPUs in the Attr.td file for explained reasons. Still negotiable :) https://reviews.llvm.org/D47474

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane added a comment. Comment at: include/clang/AST/Decl.h:2212-2213 + bool isCpuDispatchMultiVersion() const; + bool isCpuSpecificMultiVersion() const; + aaron.ballman wrote: > aaron.ballman wrote: > > Pedantic nit: CPU instead of Cpu? > Thoughts o

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

erichkeane added inline comments. Comment at: include/clang/Lex/Preprocessor.h:155 + /// A prefix map for __FILE__ and __BASEFILE__ + llvm::SmallDenseMap MacroPrefixMap; + Scrolling back up, put this implementation as close to the debug implementation as you c

[PATCH] D49504: Enable C++2a Chrono Literals

erichkeane created this revision. erichkeane added reviewers: rsmith, gribozavr, ddunbar, eli.friedman. Herald added a subscriber: cfe-commits. C++2a via http://wg21.link/p0355 permits the library literals of 'd' and 'y'. This patch enables them in the Lexer so that they can be properly parsed.

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

erichkeane added inline comments. Comment at: include/clang/Basic/Attr.td:851 + let Spellings = [Clang<"cpu_specific">]; + let Args = [VariadicIdentifierArgument<"Cpus">]; + let Subjects = SubjectList<[Function]>; aaron.ballman wrote: > erichkeane wrote: > > a

[PATCH] D49504: Enable C++2a Chrono Literals

erichkeane added a comment. In https://reviews.llvm.org/D49504#1167045, @mclow.lists wrote: > This looks ok to me - I'm assuming that somewhere there's a test for `int foo > = 0x123d`, so we're sure that didn't get broken. The hex_d SHOULD be testing that, it ensures that the RHS is an integer

[PATCH] D49504: Enable C++2a Chrono Literals

erichkeane updated this revision to Diff 156143. erichkeane added a comment. @mclow.lists s comments. https://reviews.llvm.org/D49504 Files: lib/Lex/LiteralSupport.cpp test/SemaCXX/cxx2a-user-defined-literals.cpp Index: lib/Lex/LiteralSupport.cpp ==

[PATCH] D68584: Fix Calling Convention through aliases

erichkeane created this revision. erichkeane added reviewers: rnk, pcc. Herald added a subscriber: hiraditya. Herald added a project: LLVM. r369697 changed the behavior of stripPointerCasts to no longer include aliases. However, the code in CGDeclCXX.cpp's createAtExitStub counted on the looking

[PATCH] D68584: Fix Calling Convention through aliases

This revision was automatically updated to reflect the committed changes. Closed by commit rL373929: Fix Calling Convention through aliases (authored by erichkeane, committed by ). Changed prior to commit: https://reviews.llvm.org/D68584?vs=223617&id=223750#toc Repository: rL LLVM CHANGES S

[PATCH] D68627: [Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.

erichkeane added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:947 - virtual bool validateOutputSize(StringRef /*Constraint*/, + virtual bool validateOutputSize(const llvm::StringMap &FeatureMap, + StringRef /*Constraint*

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

erichkeane created this revision. erichkeane added reviewers: lebedev.ri, hfinkel, joey, jdoerfert. lebedev.ri added a comment. Thanks, i like it. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2058-2073 +/* void CodeGenFunction::EmitAlignmentAssumption(llvm::Value *PtrValu

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

erichkeane updated this revision to Diff 224449. erichkeane added a comment. Remove unused code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68824/new/ https://reviews.llvm.org/D68824 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/CodeGen/CGBuiltin.cpp clang/l

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

erichkeane updated this revision to Diff 224455. erichkeane marked 5 inline comments as done. erichkeane added a comment. Hal's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68824/new/ https://reviews.llvm.org/D68824 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

erichkeane updated this revision to Diff 224456. erichkeane added a comment. cant use llvm::Value in Sema without some linker issues. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68824/new/ https://reviews.llvm.org/D68824 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td cla

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

erichkeane marked 5 inline comments as done. erichkeane added subscribers: majnemer, aaron.ballman. erichkeane added a comment. In D68824#1704942 , @rsmith wrote: > This seems to be missing a CodeGen test for what IR we generate on an > overly-large align

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2197 +const Expr *ProbArg = E->getArg(2); +ProbArg->EvaluateAsFloat(Probability, CGM.getContext()); +llvm::Type *Ty = ConvertType(ProbArg->getType()); You likely need to ass

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2202 +// won't use it for anything. +// Note, we still IRGen ExpectedValue because it could have side-effects. +if (CGM.getCodeGenOpts().OptimizationLevel == 0) erichkeane w

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added a comment. FYI: I'm more of a clang contributor, so I'm unable to review the LLVM code, hopefully someone will come along who can check on that. Comment at: clang/lib/Sema/SemaChecking.cpp:1805 +const Expr *ProbArg = TheCall->getArg(2); +if (ProbArg->i

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added a comment. Additionally, it seems you're missing SEMA tests. Please add those. Those tests should also make sure that the diagnostics still work with dependent values. Comment at: clang/include/clang/Basic/Builtins.def:567 BUILTIN(__builtin_expect, "LiLiLi

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added a comment. 1 more nit that I saw (don't use an 'else' after an 'if' branch that returns), but otherwise I think this is good from the CFE perspective. Someone better understanding of LLVM needs to look at the rest though. Comment at: clang/lib/Sema/SemaCheck

[PATCH] D80743: (PR46111) Desugar Elaborated types in Deduction Guides.

erichkeane added a comment. I hope that @rsmith pays particular attention here for me, I'm unable to come up with any cases where this isn't sufficient/breaks other things, but I also don't feel like I know deduction guides well enough to claim that I don't break anything :) All the lit tests

[PATCH] D80743: (PR46111) Desugar Elaborated types in Deduction Guides.

erichkeane created this revision. erichkeane added reviewers: rsmith, rjmccall. erichkeane added a comment. I hope that @rsmith pays particular attention here for me, I'm unable to come up with any cases where this isn't sufficient/breaks other things, but I also don't feel like I know deduction

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

erichkeane added a comment. 1 comment, perhaps outside the scope of this patch. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87 Ret.emplace_back("CXX11", std::string(Name), "gnu", true); + if (Spelling->getValueAsBit("AllowInC")) +Ret.emplace_back("

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

erichkeane added a comment. 1 more question, how did you arrive at the 'not supported in C' list? Did you find some GCC docs for that (if so, please put in the commit message)? Or did you just try them all? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://reviews.llv

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

erichkeane accepted this revision. erichkeane added a comment. In D80836#2064182 , @dblaikie wrote: > (sorry @erichkeane didn't mean to usurp your feedback by approving before it > was answered - @aaron.ballman do treat my approval as contingent on that

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane closed this revision. erichkeane added a comment. Herald added a subscriber: sstefan1. Yikes, I apparently misspelled "Revision" as "revisions"!. https://github.com/llvm/llvm-project/commit/8a1c999c9b0817d4de778a62965b4af86416e4b7 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

erichkeane added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65 -bool Unset; -K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset); } Doesn't this result in 'K' being uninitialized now? CHANGES SINCE LAST ACTION https

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

erichkeane added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65 -bool Unset; -K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset); } aaron.ballman wrote: > erichkeane wrote: > > Doesn't this result in 'K' being uninitializ

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane created this revision. erichkeane added reviewers: aaron.ballman, jdoerfert, jroelofs. Herald added a project: LLVM. This is a rule that seems to have been enforced for the better part of the decade, so we should document it for new contributors. Repository: rG LLVM Github Monorepo

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane marked 4 inline comments as done. erichkeane added inline comments. Comment at: llvm/docs/CodingStandards.rst:1573 +Don't Use Braces on Simple Single-Statement Bodies of if/else/loop Statements +

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane updated this revision to Diff 267714. erichkeane added a comment. Attempt improvements based on feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80947/new/ https://reviews.llvm.org/D80947 Files: llvm/docs/CodingStandards.rst Index: llvm/docs/CodingStandards.rst ===

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane updated this revision to Diff 267856. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80947/new/ https://reviews.llvm.org/D80947 Files: llvm/docs/CodingStandards.rst Index: llvm/docs/CodingStandards.rst === --- l

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane marked 6 inline comments as done. erichkeane added inline comments. Comment at: llvm/docs/CodingStandards.rst:1578 +unnecessary and otherwise meaningless code. Braces should be used however +in cases where it significantly improves readability, such as when the single

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane marked 5 inline comments as done. erichkeane added inline comments. Comment at: llvm/docs/CodingStandards.rst:1573 +Don't Use Braces on Simple Single-Statement Bodies of if/else/loop Statements +

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

erichkeane updated this revision to Diff 268175. erichkeane added a comment. Apply @hubert.reinterpretcast s spelling. I'd also like to re-enforce, the purpose of this patch is simply to write down the rule that we've been enforcing for years, I don't think we should get novel with the rules/ch

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane created this revision. erichkeane added reviewers: rsmith, rjmccall, aaron.ballman. DR2303 fixes the case where the derived-base match for template deduction is ambiguous if a base-of-base ALSO matches. The canonical example (as shown in the test) is just like the MSVC implementation of

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane added a comment. I added @rsmith directly, since I suspect he's the most knowledgeable in SemaTemplateDeduction.cpp, but if anyone knows of someone else who is a good fit for this review, please add them as well! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84048/new/ http

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane updated this revision to Diff 278825. erichkeane added a comment. Woops, left in a TODO from when I was planning on how to do this patch. Should be ready now :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84048/new/ https://reviews.llvm.org/D84048 Files: clang/lib/Sema

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane updated this revision to Diff 279598. erichkeane added a comment. Also make sure to update the the cxx_dr_status.html document. Additionally, I sent out a CWG message on the reflector about this: https://godbolt.org/z/bEr61T My implementation has this as ambiguous, but the wording ma

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane added a comment. In D84048#2165023 , @riccibruno wrote: > > Also make sure to update the the cxx_dr_status.html document. > > It is auto-generated. You need to tag the test case: `dr2303: 12`. > > Edit: And put it at the right place in the file

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane added a comment. In D84048#2165026 , @erichkeane wrote: > In D84048#2165023 , @riccibruno > wrote: > > > > Also make sure to update the the cxx_dr_status.html document. > > > > It is auto-generated. You

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane updated this revision to Diff 279609. erichkeane added a comment. Revert change to cxx_dr_status and update the test to have the right tag. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84048/new/ https://reviews.llvm.org/D84048 Files: clang/lib/Sema/SemaTemplateDeduction.

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane marked 2 inline comments as done. erichkeane added a comment. I'd attempted to regenerate the cxx_dr_status.html using make_cxx_dr_status, but I get a 'no such In D84048#2165080 , @rsmith wrote: > In D84048#2164950

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane updated this revision to Diff 279847. erichkeane marked an inline comment as done. erichkeane added a comment. As CWG seems unmoved by my argument as to why CWG2303 should change, I've opted to implement it as worded. As you can see, the implementation is somewhat more complicated, b

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

erichkeane marked 5 inline comments as done. erichkeane added a comment. Thanks for the review! I'll get this updated in the morning. I DO have a question on your suggestion for the ToVisit/Visited example, so if you could explain a little better, I'd be grateful. Comment a

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