[PATCH] D159138: [clang][Sema] Fix format size estimator's handling of %o, %x, %X with alternative form

serge-sans-paille added inline comments. Comment at: clang/docs/ReleaseNotes.rst:125 * ``-Woverriding-t-option`` is renamed to ``-Woverriding-option``. * ``-Winterrupt-service-routine`` is renamed to ``-Wexcessive-regsave`` as a generalization ??

[PATCH] D156337: [clang] Allow setting the uninitialized attribute on record

serge-sans-paille added a comment. In D156337#4558410 , @aaron.ballman wrote: > Was there an RFC for this extension to the attribute? (There doesn't need to > be one, I'm just wondering if there's more background info on what's driving > this patch

[PATCH] D159138: [clang][Sema] Fix format size estimator's handling of %o, %x, %X with alternative form

serge-sans-paille added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:966 +// size. e.g.(("%#3x", 0xf) is "0xf") + +// If the result is zero, o, b, x, X adds nothing. Then couldn't we increase the Size if the `FieldWidth` is

[PATCH] D148381: [WIP][Clang] Add counted_by attribute

serge-sans-paille added a comment. Jumping a bit late in the thread, apologize in advance if I missed something. The GCC version of the attributes seems to be `element_count` (as found in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896 mentionned by @kees) I guess we could align

[PATCH] D158558: [clang] - Add missing builtin name to AtomicExpr JSON dump

serge-sans-paille added a comment. In D158558#4616275 , @efocht wrote: > Can somebody please revert this patch? It kills all builds on > https://lab.llvm.org/buildbot/#/builders/91 since yesterday. > @serge-sans-paille ? This got reverted by

[PATCH] D158558: [clang] - Add missing builtin name to AtomicExpr JSON dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdbccf3c6f13d: [clang] - Add missing builtin name to AtomicExpr JSON dump (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo

[PATCH] D158558: [clang] - Add missing builtin name to AtomicExpr JSON dump

serge-sans-paille added a comment. I don't have a strong opinion on the synchronization with `TextNodeDumper`. I'm pretty sure I didn't update it while adding fields for the Attribute nodes though... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158558: [clang] - Add missing builtin name to AtomicExpr JSON dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As a side effect, introduce

[PATCH] D157814: [clang] Add missing field to TLSModelAttr json AST dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2105c32b32bf: [clang] Add missing field to TLSModelAttr json AST dump (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2107d87c5a91: [clang] Add missing field to VisibilityAttr json AST dump (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo

[PATCH] D157795: [clang] Add missing field to SectionAttr json AST dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4cf7666b8f09: [clang] Add missing field to SectionAttr json AST dump (authored by serge-sans-paille). Changed prior to commit:

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3dadc290b210: [clang] Add missing fields to DeprecatedAttr and Unavailable json AST dump (authored by serge-sans-paille). Repository: rG LLVM

[PATCH] D157781: [clang] Add cleanup_function field to CleanupAttr json AST dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG903e530bb898: [clang] Add cleanup_function field to CleanupAttr json AST dump (authored by serge-sans-paille). Repository: rG LLVM Github

[PATCH] D157775: [clang] Add aliasee field to AliasAttr json AST dump

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG11f3286585ce: [clang] Add aliasee field to AliasAttr json AST dump (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Review taken into account. Thanks for handling this batch of patches o/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157783/new/ https://reviews.llvm.org/D157783

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

serge-sans-paille updated this revision to Diff 549929. serge-sans-paille added a comment. Only output extra fields if they are not empty CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157783/new/ https://reviews.llvm.org/D157783 Files: clang/include/clang/AST/JSONNodeDumper.h

[PATCH] D157814: [clang] Add missing field to TLSModelAttr json AST dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

serge-sans-paille updated this revision to Diff 549717. serge-sans-paille added a comment. Fix typo in example CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157808/new/ https://reviews.llvm.org/D157808 Files: clang/include/clang/AST/JSONNodeDumper.h

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D157795: [clang] Add missing field to SectionAttr json AST dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

serge-sans-paille updated this revision to Diff 549645. serge-sans-paille added a comment. + test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157783/new/ https://reviews.llvm.org/D157783 Files: clang/include/clang/AST/JSONNodeDumper.h clang/lib/AST/JSONNodeDumper.cpp

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

serge-sans-paille updated this revision to Diff 549644. serge-sans-paille retitled this revision from "[clang] Add reason and replacement fields to DeprecatedAttr json AST dump" to "[clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump". serge-sans-paille added a

[PATCH] D157783: [clang] Add reason and replacement fields to DeprecatedAttr json AST dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D157781: [clang] Add cleanup_function field to CleanupAttr json AST dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D157775: [clang] Add aliasee field to AliasAttr json AST dump

serge-sans-paille created this revision. serge-sans-paille added a reviewer: aaron.ballman. Herald added a subscriber: jeroen.dobbelaere. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D156337: [clang] Allow setting the uninitialized attribute on record

serge-sans-paille updated this revision to Diff 545074. serge-sans-paille added a comment. (rebased on `main`) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156337/new/ https://reviews.llvm.org/D156337 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td

[PATCH] D156337: [clang] Allow setting the uninitialized attribute on record

serge-sans-paille updated this revision to Diff 545060. serge-sans-paille added a comment. Take reviewers comment into account. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156337/new/ https://reviews.llvm.org/D156337 Files: clang/include/clang/Basic/Attr.td

[PATCH] D156337: [clang] Allow setting the uninitialized attribute on record

serge-sans-paille created this revision. serge-sans-paille added reviewers: jfb, aaron.ballman. Herald added a subscriber: jdoerfert. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

serge-sans-paille abandoned this revision. serge-sans-paille added a comment. gcc bug filled as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110816 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155580/new/ https://reviews.llvm.org/D155580

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

serge-sans-paille added a subscriber: jakubjelinek. serge-sans-paille added a comment. @jfb ok, makes sense. For the record it does show up in our profile but I have been able to workaround it without touching the compiler. This probably mean that the GCC implementation is having a small hole,

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

serge-sans-paille updated this revision to Diff 541475. serge-sans-paille added a comment. switch from cascading ternary operator to cascading if for readability. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155580/new/ https://reviews.llvm.org/D155580 Files:

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

serge-sans-paille added a comment. Comparison with GCC behavior here: https://godbolt.org/z/xc3j9jEsf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155580/new/ https://reviews.llvm.org/D155580 ___

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

serge-sans-paille created this revision. serge-sans-paille added reviewers: nickdesaulniers, jfb, aaron.ballman. Herald added a subscriber: kristof.beyls. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

This revision was automatically updated to reflect the committed changes. Closed by commit rGdc6f5c9b588a: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align (authored by serge-sans-paille). Changed prior to commit: https://reviews.llvm.org/D151753?vs=528999=534869#toc

[PATCH] D153746: [Remarks] Make sure -fdiagnostics-hotness-threshold implies -fdiagnostics-show-hotness

This revision was automatically updated to reflect the committed changes. Closed by commit rGf686e4bbc20a: [Remarks] Make sure -fdiagnostics-hotness-threshold implies -fdiagnostics-show… (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153746: [Remarks] Make sure -fdiagnostics-hotness-threshold implies -fdiagnostics-show-hotness

serge-sans-paille created this revision. serge-sans-paille added a reviewer: weiwang. Herald added a subscriber: hiraditya. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits.

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

serge-sans-paille added a comment. Gentle ping :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151753/new/ https://reviews.llvm.org/D151753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D152771: [docs] Fix LibTooling documentation for OptionsParser change

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. I'm fine with the change. It would be great if this could `literalinclude` these two sample codes (as in

[PATCH] D152396: [clang][doc] Rescue some deleted bits of the command-line reference.

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. Another argument for not having generated file in-tree. Thanks for doing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D151594: [clang-tidy] Optimize misc-confusable-identifiers

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. LGTM with a minor nit. Thanks for the optimization! Comment at: clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp:124 +if

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

serge-sans-paille added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:16163 +// Dependent alignment cannot be resolved -> bail out. +if (any_of(VD->specific_attrs(), + [](auto *A) { return A->isAlignmentDependent(); }))

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

serge-sans-paille added a comment. In D151753#4383602 , @Fznamznon wrote: > Have you checked that it fixes the non-reduced original example? Just did and it passes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151753/new/

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

serge-sans-paille updated this revision to Diff 528999. serge-sans-paille added a comment. Take review into account CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151753/new/ https://reviews.llvm.org/D151753 Files: clang/lib/Sema/SemaChecking.cpp

[PATCH] D151783: [clang] Use the appropriate definition when checking FunctionDecl::isInlineBuiltinDeclaration

This revision was automatically updated to reflect the committed changes. Closed by commit rG1676b1ff63af: [clang] Use the appropriate definition when checking FunctionDecl… (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

serge-sans-paille added a comment. Companion patch that fixes the issue reported by @mstorsjo : https://reviews.llvm.org/D151783 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148723/new/ https://reviews.llvm.org/D148723

[PATCH] D151783: [clang] Use the appropriate definition when checking FunctionDecl::isInlineBuiltinDeclaration

serge-sans-paille created this revision. serge-sans-paille added a reviewer: mstorsjo. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a follow-up to https://reviews.llvm.org/D148723

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

serge-sans-paille created this revision. serge-sans-paille added reviewers: dim, Fznamznon. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix #63007 Repository: rG LLVM Github Monorepo

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

serge-sans-paille added a comment. In D148723#4380165 , @mstorsjo wrote: > This causes failed asserts with `_FORTIFY_SOURCE` with the mingw-w64 headers. > Here's a reduced reproducer: > > $ cat reduced.c > typedef unsigned int size_t; > > void

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

This revision was automatically updated to reflect the committed changes. Closed by commit rGe20931b2ccd1: [clang] Restrict Inline Builtin to non-static, non-odr linkage (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

serge-sans-paille added a comment. @jyu2 : could you test that patch in your setup ? Also I can't find the bug id associated with your original bug anymore... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148723/new/ https://reviews.llvm.org/D148723

[PATCH] D148723: [clang] Restrict Inline Builtin to non-static, non-odr linkage

serge-sans-paille updated this revision to Diff 526059. serge-sans-paille retitled this revision from "[clang] Fix comdat for InlineBuiltin declarations" to "[clang] Restrict Inline Builtin to non-static, non-odr linkage". serge-sans-paille edited the summary of this revision. serge-sans-paille

[PATCH] D151051: [clang-tidy] Optimize misc-confusable-identifiers

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. Memoizing the calls to getPrimaryContext()... LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151051/new/

[PATCH] D148723: [clang] Fix comdat for InlineBuiltin declarations

serge-sans-paille updated this revision to Diff 523747. serge-sans-paille retitled this revision from "[clang] Enforce external linkage for inline builtin original declaration" to "[clang] Fix comdat for InlineBuiltin declarations". serge-sans-paille edited the summary of this revision. CHANGES

[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

serge-sans-paille added a comment. > So if isInlineBuiltinDeclaration() simply returns false for C++ code, > everything should just work, without messing with the linkage. Unfortunately not. See the example from `clang/test/CodeGen/inline-builtin-comdat.c` above. There's no C++ involved, the

[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

serge-sans-paille added a comment. In D148723#4296111 , @efriedma wrote: > I'm a little concerned that this will explode in unexpected ways... in > particular, it'll fail to link if the function doesn't actually exist > externally. Which it probably

[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

serge-sans-paille added a comment. In D148723#4296112 , @efriedma wrote: > Really, I'd prefer to keep isInlineBuiltinDeclaration() targeted as narrowly > as possible; part of that is making it not trigger for C++ inline functions > (which it never did

[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

serge-sans-paille updated this revision to Diff 516645. serge-sans-paille retitled this revision from "[clang] Enforce internal linkage for inline builtin" to "[clang] Enforce external linkage for inline builtin original declaration". serge-sans-paille edited the summary of this revision.

[PATCH] D148723: [clang] Enforce internal linkage for inline builtin

serge-sans-paille added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5192 - if (const auto *FD = D->getAsFunction()) + if (const auto *FD = D->getAsFunction()) { if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally)

[PATCH] D148723: [clang] Enforce internal linkage for inline builtin

serge-sans-paille added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:11538 const FunctionDecl *FD) { - if (!FD->isExternallyVisible()) + if (!FD->isExternallyVisible() || FD->isInlineBuiltinDeclaration()) return

[PATCH] D148723: [clang] Enforce internal linkage for inline builtin

serge-sans-paille added a comment. In D148723#4280916 , @efriedma wrote: > This seems like a weird way to fix this. I agree, not a big fan either. But wanting to start the bike shedding in some way. > The point of an "inline builtin" is that the

[PATCH] D148723: [clang] Enforce internal linkage for inline builtin

serge-sans-paille created this revision. serge-sans-paille added reviewers: jyu2, efriedma. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Otherwise we end up with link once odr because of

[PATCH] D147307: [clang] Do not require GNUInlineAttr for inline builtins

serge-sans-paille added a comment. In D147307#4279337 , @jyu2 wrote: > This change seems cause assert: > > https://godbolt.org/z/z9G87Wr37 I can reproduce, investigating... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D147307: [clang] Do not require GNUInlineAttr for inline builtins

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd8745c252bc: [clang] Do not require GNUInlineAttr for inline builtins (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D147307: [clang] Do not require GNUInlineAttr for inline builtins

serge-sans-paille updated this revision to Diff 510752. serge-sans-paille retitled this revision from "[clang] Consider artificial always inline builtin as inline builtins" to "[clang] Do not require GNUInlineAttr for inline builtins". serge-sans-paille added a comment. DO not require gnu

[PATCH] D147307: [clang] Consider artificial always inline builtin as inline builtins

serge-sans-paille created this revision. serge-sans-paille added reviewers: aaron.ballman, tstellar, efriedma. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix #61691 Repository: rG

[PATCH] D140075: [libclang] Expose using shadow types and declarations in libclang.

serge-sans-paille added inline comments. Comment at: clang/tools/libclang/CXCursor.cpp:1281 +if (const UsingShadowDecl *Shadow = Using->getFoundDecl()) + if (const auto *TD = dyn_cast_or_null(Shadow->getTargetDecl())) +return MakeCursorTypeRef(TD, Loc, TU);

[PATCH] D140074: [libclang] Remove redundant return statements in CXType.cpp

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5c943e368818: [libclang] Remove redundant return statements in CXType.cpp (authored by emilio, committed by serge-sans-paille). Repository: rG

[PATCH] D140074: [libclang] Remove redundant return statements in CXType.cpp

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. The cleanup looks good to me, thanks! Out of curiosity, did just stumble on this, or did an automatic tool gave you an hint? Repository: rG LLVM Github Monorepo

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG0ffaffcaac97: Reapply 6fa2abf90886f18472c87bc9bffbcdf4f73c465e (authored by serge-sans-paille). Repository: rG LLVM

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6fa2abf90886: Lazyly initialize uncommon toolchain detector

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

serge-sans-paille updated this revision to Diff 494927. serge-sans-paille added a comment. Address @tbader's comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142606/new/ https://reviews.llvm.org/D142606 Files: clang/lib/Driver/ToolChains/AMDGPU.cpp

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

serge-sans-paille added inline comments. Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26 + + mutable std::optional Detector; + zero9178 wrote: > serge-sans-paille wrote: > > zero9178 wrote: > > > Any reason this is `mutable`? I don't see it being used

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

serge-sans-paille updated this revision to Diff 494877. serge-sans-paille added a comment. remove mutable qualifier CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142606/new/ https://reviews.llvm.org/D142606 Files: clang/lib/Driver/ToolChains/AMDGPU.cpp

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

serge-sans-paille added a comment. Gentle ping :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142181/new/ https://reviews.llvm.org/D142181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

serge-sans-paille added inline comments. Comment at: clang/lib/Driver/ToolChains/LazyDetector.h:26 + + mutable std::optional Detector; + zero9178 wrote: > Any reason this is `mutable`? I don't see it being used a non-const way in a > const method yeah, `T

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

serge-sans-paille added a comment. https://llvm-compile-time-tracker.com/compare.php?from=1a17739d3aa78599c32f6106e05dcfa7f3f9e823=9c1adc5fcca177d6628bdb22c72e7546c7b691aa=instructions:u looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

serge-sans-paille added a comment. Do you intend to (optionnaly) provide XXH3 as described in https://github.com/Cyan4973/xxHash ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142862/new/ https://reviews.llvm.org/D142862

[PATCH] D142862: [Support] change StringMap hash function from djbHash to xxHash

serge-sans-paille added a comment. Can you take a shot against https://llvm-compile-time-tracker.com/ so that we get an hint of the practical speedup? Looks like xxhash.h is missing from the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

serge-sans-paille added a comment. In D142606#4082753 , @tstellar wrote: > Does this mean that clang will no longer search for the ROCM and CUDA library > paths for every C compile? That's the goal, yes. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D142606: Lazyly initialize uncommon toolchain detector

serge-sans-paille created this revision. Herald added subscribers: kosarev, kerbowa, jvesely, emaste. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Cuda and rocm toolchain detectors

[PATCH] D142024: [clang] Optimize clang::Builtin::Info density

serge-sans-paille added a comment. This got fixed in d1f313e73a1b33d662dbab54be76720b9c875645 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142024/new/

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

serge-sans-paille updated this revision to Diff 491400. serge-sans-paille added a comment. Do not enfore StringLiteral parameter. This is too invasive on the code base. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142181/new/ https://reviews.llvm.org/D142181 Files:

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

serge-sans-paille added a comment. In D142181#4070881 , @nikic wrote: > In D142181#4070868 , > @serge-sans-paille wrote: > >> @nikic : I thought I fixed that with >> e8a163dc03e6913360beb305620104ba129c081c >>

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

serge-sans-paille added a comment. > Plausibly caused by this change -- I'm thinking that `MaximumDistance` is > initialized to `UINT_MAX - 1`, but then we cast that to a signed integer and > compare that, so it wraps to a large negative number. @nikic : I thought I fixed that with

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

serge-sans-paille added a comment. @nikic : I thought I fixed that with e8a163dc03e6913360beb305620104ba129c081c ... is it included in your build? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142181/new/

[PATCH] D142181: [OptTable] Store llvm::cl::Option into a DenseMap instead of a StringMap

serge-sans-paille updated this revision to Diff 490804. serge-sans-paille added a comment. Herald added subscribers: cfe-commits, kadircet, arphaman. Herald added a project: clang-tools-extra. Fix build + make StringLiteral initialization constexpr. CHANGES SINCE LAST ACTION

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6ad1b4095172: Optimize OptTable::findNearest implementation and usage (authored by serge-sans-paille). Changed prior to commit:

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

serge-sans-paille updated this revision to Diff 490412. serge-sans-paille added a comment. Thanks @nikic for the review. Remarks taken into account. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142026/new/ https://reviews.llvm.org/D142026 Files: clang/lib/Driver/Driver.cpp

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

serge-sans-paille created this revision. serge-sans-paille added reviewers: aaron.ballman, nikic. Herald added subscribers: StephenFan, hiraditya. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, MaskRay. Herald

[PATCH] D141814: [llvm][ADT] Replace uses of `makeMutableArrayRef` with deduction guides

serge-sans-paille added a comment. LGTM Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:652 auto data = + llvm::MutableArrayRef(static_cast(buf), bytes_read); random nit: This could be rewritten as ``` llvm::MutableArrayRef

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG07bb29d8ffc3: [OptTable] Precompute OptTable prefixes union table through tablegen (authored by serge-sans-paille). Repository: rG LLVM Github

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbbe463d6ba26: [OptTable] Make ValuesCode initialisation of Options constexpr (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

serge-sans-paille updated this revision to Diff 488348. serge-sans-paille added a comment. nits + rebased on main CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140699/new/ https://reviews.llvm.org/D140699 Files: clang/include/clang/Driver/Options.td

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

serge-sans-paille marked 2 inline comments as done. serge-sans-paille added a comment. In D140800#4043723 , @nikic wrote: > Just to check, this isn't going to cause some warning spew about all those > OptTable implementations being non-final? nope. Why

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

serge-sans-paille updated this revision to Diff 488344. serge-sans-paille added a comment. Nits + rebase on main branch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140800/new/ https://reviews.llvm.org/D140800 Files: clang/lib/Driver/DriverOptions.cpp

[PATCH] D141047: build: with -DCLANGD_ENABLE_REMOTE=ON, search for grpc++ dependencies too

serge-sans-paille accepted this revision. serge-sans-paille added a comment. This revision is now accepted and ready to land. LGTM, assuming, you actually tested that locally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141047/new/

[PATCH] D140800: [OptTable] Precompute OptTable prefixes union table through tablegen

serge-sans-paille added a comment. @nikic : gentle ping :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140800/new/ https://reviews.llvm.org/D140800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D140699: [OptTable] Make ValuesCode initialisation of Options constexpr

serge-sans-paille added a comment. Herald added a subscriber: StephenFan. @nikic: gentle ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140699/new/ https://reviews.llvm.org/D140699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D141298: Move from llvm::makeArrayRef to ArrayRef deduction guides - last part

serge-sans-paille added a comment. Herald added subscribers: Michael137, JDevlieghere. Once that patch lands, I'll mark `makeArrayRef` as deprecated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141298/new/ https://reviews.llvm.org/D141298

[PATCH] D141047: build: with -DCLANGD_ENABLE_REMOTE=ON, search for grpc++ dependencies too

serge-sans-paille added inline comments. Comment at: cmake/Modules/FindGRPC.cmake:85 if(NOT TARGET grpc++) +find_library(GPR_LIBRARY gpr $GRPC_OPTS REQUIRED) +add_library(gpr UNKNOWN IMPORTED GLOBAL) Shouldn't this be `${GPRC_OPTS}`?

  1   2   3   4   5   6   7   8   9   >