[PATCH] D129572: [X86] initial -mfunction-return=thunk-extern support

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:42 +// CHECK: @double_keep_thunk2() [[EXTERN]] +[[gnu::function_return("thunk-keep")]][[gnu::function_return("thunk-extern")]] +void double_keep_thunk2(void) {} I just no

[PATCH] D129691: [clang][test] fix typo in fn attr

nickdesaulniers created this revision. nickdesaulniers added reviewers: aaron.ballman, erichkeane, MaskRay. Herald added subscribers: jsji, StephenFan, pengfei. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D129691: [clang][test] fix typo in fn attr

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:18 // CHECK: @keep2() [[KEEP:#[0-9]+]] [[gnu::function_return("keep")]] void keep2(void) {} probably don't want to reset `KEEP` Comment at: clang/t

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers created this revision. nickdesaulniers added reviewers: MaskRay, void. Herald added subscribers: jsji, StephenFan, jdoerfert, pengfei, hiraditya. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added projects: clang, LLVM. Herald added subscrib

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:107 +// CHECK-GCOV: @__llvm_gcov_init() unnamed_addr [[EXTERNGCOV]] +// CHECK-ASAN: @asan.module_ctor() [[EXTERNASAN:#[0-9]+]] +// CHECK-TSAN: @tsan.module_ctor() [[EXTERNTSAN:#[0-9]+]] -

[PATCH] D129691: [clang][test] fix typo in fn attr

nickdesaulniers updated this revision to Diff 444695. nickdesaulniers added a comment. - don't reset captured vars, that was a copy+paste mistake I made in the original patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129691/new/ https://review

[PATCH] D129691: [clang][test] fix typo in fn attr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd2792e7d37c4: [clang][test] fix typo in fn attr (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers updated this revision to Diff 444725. nickdesaulniers added a comment. - rebase on top of D129691 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129709/new/ https://reviews.llvm.org/D129709 Files:

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers updated this revision to Diff 444742. nickdesaulniers marked 2 inline comments as done. nickdesaulniers added a comment. - rename md idententifier, sort langref Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129709/new/ https://revie

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers added a comment. Thanks for the review! Comment at: clang/test/CodeGen/attr-function-return.c:9 // RUN: --check-prefixes=CHECK,CHECK-EXTERN +// RUN: %clang_cc1 -std=gnu2x -triple x86_64-linux-gnu %s -emit-llvm -o - \ +// RUN: -mfunction-return=thunk-extern -f

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers updated this revision to Diff 444743. nickdesaulniers added a comment. - resort langref, oops Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129709/new/ https://reviews.llvm.org/D129709 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG140bfdca60ae: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D129709: [clang][CodeGen] add fn_ret_thunk_extern to synthetic fns

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/attr-function-return.c:9 // RUN: --check-prefixes=CHECK,CHECK-EXTERN +// RUN: %clang_cc1 -std=gnu2x -triple x86_64-linux-gnu %s -emit-llvm -o - \ +// RUN: -mfunction-return=thunk-extern -fprofile-arcs \

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. It looks like clang still produces meaningful diagnostics from `-Wuninitialized` and `-Wsometimes-uninitialized` even with `-ftrivial-auto-var-init=zero`. Same for `[clang-

[PATCH] D95408: [Sema][C] members of anonymous struct inherit QualType

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. Herald added a project: All. Prefer https://reviews.llvm.org/D125167. Comment at: clang/test/AST/ast-dump-decl.c:132 +// CHECK: IndirectFieldDecl{{.*}} Test48755Const 'int' +// CHECK-NEXT: Field{{.*}} '

[PATCH] D125167: [WIP] Fix member access of anonymous struct/union fields in C

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Consider adding an AST test; that was something that came up in my patch https://reviews.llvm.org/D95408. > I'm pushing back on the WG14 lists to see if this is a good opport

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Worth mentioning the deprecation plans in `clang/docs/ReleaseNotes.rst` and updaing the commit message+description on phab? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12514

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers added a comment. I still think this would be easier to review if the `isArgumentRegister` tablegen changes were separated out into a distinct parent patch and then the existing x86 implementation updated to use, then this would rebased on top of as a child patch.

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers added a comment. In D124836#3507268 , @void wrote: > I'll split off the TableGen changes into a separate patch. It will supersede > those changes here, so it shouldn't delay other reviews here. :-) I'm referring to the changes to llvm/ut

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t + Consider adding a test: ``` // RUN: %clang_cc1 -fsyntax-only -verify %s -x assembler-with-cpp < %s ```

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:484 const SourceLocation &EndLoc) const { + // If this is a `.S` file, treat unknown # directives as non-preprocessor + // directives. A

[PATCH] D125727: Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t + nickdesaulniers wrote: > Consider adding a test: > > ``` > // RUN: %clang_cc1 -fsyntax-only -verify %s

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify -S %s -o %t + nickdesaulniers wrote: > nickdesaulniers wrote: > > Consider adding a test: > > > > ``` > > // RUN: %c

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added inline comments. Comment at: clang/test/Preprocessor/suggest-typoed-directive.S:1 +// RUN: %clang_cc1 -fsyntax-only -verify %s - < %s + These three can be removed now as well. Clang will read `%s` as input without the need to read from std

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. > I do not have permission to land this patch, so could you please do it on my > behalf? Will do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727 ___

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. Sorry, it looks like `arcanist` or my PHP runtime was auto updated on my host and has regressed. I need to sort that out first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D12

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. sorted, will commit soon Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG45e01ce5fe6a: [clang] Avoid suggesting typoed directives in `.S` files (authored by ken-matsui, committed by nickdesaulniers). Repository: rG LLVM

[PATCH] D125727: [clang] Avoid suggesting typoed directives in `.S` files

nickdesaulniers added a comment. pushed 45e01ce5fe6a5e4dc25ffdf626caa344fbcb93dd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125727/new/ https://reviews.llvm.org/D125727

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Don't forget to rebase this on top of https://reviews.llvm.org/D125421. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124836/ne

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers added a comment. Thanks for the patch! > A tangential question is do we need these two options expect all? For the Linux kernel's use, no. But I think it would extremely simple to implement. Instead of having one `SubtargetFeature` (`FeatureHardenSlsAll`), you'd have two (say `

[PATCH] D126137: [X86] Add support for `-mharden-sls=all`

nickdesaulniers added a comment. In D126137#3530777 , @kristof.beyls wrote: > Therefore, I wonder if it wouldn't be better to name this -mharden-sls=retbr > for more consistency across architectures? I think it's best to maintain compatibility with GCC

[PATCH] D66492: [Clang][CodeGen] set alias linkage on QualType

nickdesaulniers created this revision. nickdesaulniers added a reviewer: rsmith. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar. Herald added a project: clang. It seems that CodeGen was always using ExternalLinkage when emitting a GlobalDecl with __attribute__((alias)). This lea

[PATCH] D66492: [Clang][CodeGen] set alias linkage on QualType

nickdesaulniers updated this revision to Diff 216697. nickdesaulniers added a comment. - add unit test for function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66492/new/ https://reviews.llvm.org/D66492 Files: clang/lib/CodeGen/CodeGenModule.c

[PATCH] D66492: [Clang][CodeGen] set alias linkage on QualType

This revision was automatically updated to reflect the committed changes. Closed by commit rL369705: [Clang][CodeGen] set alias linkage on QualType (authored by nickdesaulniers, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:8101 +// All further checking is done on the subexpression +Match = AT.matchesType(S.Context, ExprTy); +if (Match) { Maybe leave the top level Match const a

[PATCH] D66622: [Bugfix] fix r369705 unit test

nickdesaulniers created this revision. nickdesaulniers added reviewers: leonardchan, erik.pilkington. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Aliases aren't supported on OSX. Add a GNU target triple. Reported-by: leonardchan Reported-by: erik.pilkington

[PATCH] D66492: [Clang][CodeGen] set alias linkage on QualType

nickdesaulniers added a comment. @leonardchan thanks for the report, would you mind reviewing: https://reviews.llvm.org/D66622? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66492/new/ https://reviews.llvm.org/D66492 ___

[PATCH] D66622: [Bugfix] fix r369705 unit test

This revision was automatically updated to reflect the committed changes. Closed by commit rL369713: [Bugfix] fix r369705 unit test (authored by nickdesaulniers, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:8107 +return true; + Pedantic |= ImplicitMatch == analyze_printf::ArgType::NoMatchPedantic; +} At this point `ImplicitMatch` can only have the value

[PATCH] D66186: [Sema] Don't warn on printf('%hd', [char]) (PR41467)

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Thanks for being responsive to all this code review! 💃🏽 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66186/new/ https://reviews.llvm.org/D66186 ___

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added a comment. Cool, I look forward to checking for outdated `__builtin_expects` in the Linux kernel, if they exist! Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:108 + auto *L = mdconst::dyn_extract(MisExpectData->getOperand(2)); + auto *U =

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:108 + // extract values from misexpect metadata + const ConstantInt *IndexCint = + mdconst::dyn_extract(MisExpectData->getOperand(1)); `const auto *Index

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3451 +const std::vector &warnings = + Res.getDiagnosticOpts().Warnings; The indentation here looks funny. Can you please run `git-clang-format` and commit the cha

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150 +// Operand 0 is a string tag "branch_weights" +if (MDString *Tag = cast(MD->getOperand(0))) { + unsigned NOps = MD->getNumOperands(); Sorry if I'm going b

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150 +// Operand 0 is a string tag "branch_weights" +if (MDString *Tag = cast(MD->getOperand(0))) { + unsigned NOps = MD->getNumOperands(); paulkirth wrote: > n

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150 +// Operand 0 is a string tag "branch_weights" +if (MDString *Tag = cast(MD->getOperand(0))) { + unsigned NOps = MD->getNumOperands(); paulkirth wrote: > n

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers added a comment. Looks good, will likely approve after these 2 questions. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:157-158 +// In these cases we should not emit any diagnostic related to misexpect. +if (NOps < 3) + return; + -

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Ok, thanks for the iteration on code review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org

[PATCH] D67253: clang-misexpect: a standalone tool for verifying the use of __builtin_expect with PGO data

nickdesaulniers added inline comments. Comment at: clang-tools-extra/clang-misexpect/tool/ClangMisExpectMain.cpp:33 +using namespace clang::tooling; +using namespace clang::misexpect; +using Path = std::string; The `using` statements here should match clang-tool

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

nickdesaulniers added a comment. In D68101#1802220 , @bd1976llvm wrote: > Below is the code comment from the new patch explaining the new approach, > please take a look and see if you have any questions/comments: > > // If two globals with differing siz

[PATCH] D69876: Allow output constraints on "asm goto"

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. No, thanks for the work on this @void ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D698

[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

nickdesaulniers added inline comments. Comment at: clang/lib/Analysis/UninitializedValues.cpp:856 +vals[VD] = MayUninitialized; } Can you walk me through the logic of this function? I would assume for changes to `asm goto`, the above early `return` wo

[PATCH] D72221: Support function attribute patchable_function_entry

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925 + T.getArch() != llvm::Triple::x86_64) { +S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL; +return; Why is the target arch also checked in `clang/li

[PATCH] D72221: Support function attribute patchable_function_entry

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4925 + T.getArch() != llvm::Triple::x86_64) { +S.Diag(getAttrLoc(AL), diag::err_attribute_unsupported) << AL; +return; ---

[PATCH] D72221: Support function attribute patchable_function_entry

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:826 + "patchable-function-entry", + (Twine(Attr->getSize()) + "," + Twine(Attr->getStart())).str()); } ostannard wrote: > I think using two function attr

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

nickdesaulniers added a comment. In D68101#1806213 , @rjmccall wrote: > In D68101#1806135 , @nickdesaulniers > wrote: > > > In D68101#1802280 , @rjmccall > > wrote: > > > >

[PATCH] D72221: Support function attribute patchable_function_entry

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Thanks for all of the work that went into this. Looks like review comments have all been addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

nickdesaulniers added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2370 +* ``bcmp`` * ``memchr`` nickdesaulniers wrote: > rsmith wrote: > > aaron.ballman wrote: > > > Can you mention the deprecation issue here? > > Do we really provide consta

[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

nickdesaulniers added a comment. I don't have any thoughts on the change per se, so just minor thoughts/generic code review. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6995 + case llvm::Triple::aarch64_be: +if (Arg *A = Args.getLastArg(options::OPT_mmark_bti_proper

[PATCH] D87956: [WIP][IR] add fn attr for no_stack_protector; prevent inlining ssp into no-ssp

nickdesaulniers created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, hiraditya. Herald added a reviewer: jdoerfert. Herald added projects: clang, LLVM. nickdesaulniers requested review of this revision. It's currently ambiguous in IR whether the sourc

[PATCH] D87956: [WIP][IR] add fn attr for no_stack_protector; prevent inlining ssp into no-ssp

nickdesaulniers planned changes to this revision. nickdesaulniers added a comment. Not ready for review yet, posting a WIP fix for https://bugs.llvm.org/show_bug.cgi?id=47479. TODO next week: - tests, tests, and more tests. - interaction with `__attribute__((always_inline))`. - fix TODOs in cod

[PATCH] D87956: [WIP][IR] add fn attr for no_stack_protector; prevent inlining ssp into no-ssp

nickdesaulniers updated this revision to Diff 293559. nickdesaulniers added a comment. Herald added a reviewer: whitequark. Herald added a reviewer: aaron.ballman. - add tests, make fn attrs mutually exclusive Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D87956: [WIP][IR] add fn attr for no_stack_protector; prevent inlining ssp into no-ssp

nickdesaulniers added a comment. I obviously have a few todos left to resolve, but CC'ing reviewers now for general feedback. One thing I'm unsure of; it felt curious to me that nothing was enforcing that these different levels of stack protection were mutually exclusive. I've added that here

[PATCH] D88195: Remove stale assert.

nickdesaulniers added a comment. I'm super confused between the commit message and initial hunk, that seem to make sense and probably should go in clang-11 if it's not too late, and the additional tests for modules which the commit message does not address. Were these meant to be separate comm

[PATCH] D88195: Remove stale assert.

nickdesaulniers added a comment. In D88195#2293533 , @void wrote: > In D88195#2293165 , @nickdesaulniers > wrote: > >> I'm super confused between the commit message and initial hunk, that seem to >> make sense and

[PATCH] D88195: Remove stale assert.

nickdesaulniers added a comment. In D88195#2293559 , @nickdesaulniers wrote: > In D88195#2293533 , @void wrote: > >> In D88195#2293165 , >> @nickdesaulniers wrote: >> >>> I

[PATCH] D88195: Remove stale assert.

nickdesaulniers added a comment. In D88195#2293589 , @void wrote: > Clarify commit message. Phabricator unfortunately won't amend the review description when the commit message is updated; you'll need to manually edit the description via phab's UI for o

[PATCH] D88195: Remove stale assert.

nickdesaulniers accepted this revision. nickdesaulniers added a comment. In D88195#2293637 , @jyknight wrote: > In D88195#2293597 , @nickdesaulniers > wrote: > >> In D88195#2293589

[PATCH] D87956: [WIP][IR] add fn attr for no_stack_protector; prevent inlining ssp into no-ssp

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/stack-protector.c:39 // SAFESTACK-NOSSP: attributes #[[A]] = {{.*}} safestack -// SAFESTACK-NOSSP-NOT: ssp +// SAFESTACK-NOSSP-NOT: attribute #[[A]] = {{.*}} ssp should be `attributes` plura

[PATCH] D85692: python bindings: fix DeprecationWarning

nickdesaulniers added a comment. In D85692#2227531 , @jdenny wrote: > Does this mean we officially no longer support python 2, which this change > breaks? Send a patch to support both and I'll approve it. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D86508: [clang] consistently use getLangOpts()

nickdesaulniers created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. nickdesaulniers requested review of this revision. File was inconsistent. [clang] implement __builtin_puts More importantly, start adding missing tests for __builtin_*/__has_builtin, and

[PATCH] D86508: [clang] consistently use getLangOpts()

nickdesaulniers updated this revision to Diff 287572. nickdesaulniers added a comment. - add more docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86508/new/ https://reviews.llvm.org/D86508 Files: clang/docs/LanguageExtensions.rst clang/test

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers updated this revision to Diff 287573. nickdesaulniers added a comment. - misuploaded Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86508/new/ https://reviews.llvm.org/D86508 Files: clang/docs/LanguageExtensions.rst clang/includ

[PATCH] D69925: remove redundant LLVM version from version string when setting CLANG_VENDOR

nickdesaulniers added a comment. Sorry! Thanks for fixing up OpenSSL! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69925/new/ https://reviews.llvm.org/D69925 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers added a comment. bumping for review. Are there additional reviewers we could add to share the burden? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86508/new/ https://reviews.llvm.org/D86508 ___

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:513 BUILTIN(__builtin_printf, "icC*.", "Fp:0:") +BUILTIN(__builtin_putchar, "ii", "F") +BUILTIN(__builtin_puts, "icC*", "nF") rsmith wrote: > aaron.ballman wrote: > > Shoul

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers updated this revision to Diff 289312. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. - rebase on master, precommitted getLangOpts() change, dropped p7.cpp test change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:513 BUILTIN(__builtin_printf, "icC*.", "Fp:0:") +BUILTIN(__builtin_putchar, "ii", "F") +BUILTIN(__builtin_puts, "icC*", "nF") nickdesaulniers wrote: > rsmith wrote: > > aar

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:513 BUILTIN(__builtin_printf, "icC*.", "Fp:0:") +BUILTIN(__builtin_putchar, "ii", "F") +BUILTIN(__builtin_puts, "icC*", "nF") nickdesaulniers wrote: > nickdesaulniers wrote

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:513 BUILTIN(__builtin_printf, "icC*.", "Fp:0:") +BUILTIN(__builtin_putchar, "ii", "F") +BUILTIN(__builtin_puts, "icC*", "nF") nickdesaulniers wrote: > nickdesaulniers wrote

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers updated this revision to Diff 289572. nickdesaulniers added a comment. - add putc and fputc builtins Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86508/new/ https://reviews.llvm.org/D86508 Files: clang/docs/LanguageExtensions.rs

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers marked 2 inline comments as done. nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:513 BUILTIN(__builtin_printf, "icC*.", "Fp:0:") +BUILTIN(__builtin_putchar, "ii", "F") +BUILTIN(__builtin_puts, "icC*", "nF") -

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

nickdesaulniers planned changes to this revision. nickdesaulniers added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2422 +* ``vprintf`` +* ``vsprintf`` + Let me triple check the docs look good. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

nickdesaulniers updated this revision to Diff 289594. nickdesaulniers added a comment. - improve docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86508/new/ https://reviews.llvm.org/D86508 Files: clang/docs/LanguageExtensions.rst clang/inclu

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

nickdesaulniers added a comment. Ok, this is ready for review. I plan to review the newer C standards than ISO C90 in follow up patches, so I'll likely be touching these file more. Figuring out a more maintainable sort order in particular is an itch I'd like to scratch. Repository: rG LLVM

[PATCH] D92846: [KernelAddressSanitizer] Fix globals exclusion for indirect aliases

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Thanks for the patch. Comment at: clang/test/CodeGen/asan-globals-alias.cpp:30 +// KASAN: @aliased_global_2{{.*}} global i32 +// KASAN: @joydev_ids{{.*}} global [1 x {{.*}}i64 1234 }], align 16 -

[PATCH] D92846: [KernelAddressSanitizer] Fix globals exclusion for indirect aliases

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/asan-globals-alias.cpp:30 +// KASAN: @aliased_global_2{{.*}} global i32 +// KASAN: @joydev_ids{{.*}} global [1 x {{.*}}i64 1234 }], align 16 melver wrote: > nickdesaulniers wrote: > > Do we w

[PATCH] D68410: [AttrDocs] document always_inline

This revision was automatically updated to reflect the committed changes. Closed by commit rGe75fec2b238f: [AttrDocs] document always_inline (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68410/new/ https://reviews.llvm

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

nickdesaulniers added a comment. In D90180#2357247 , @aaron.ballman wrote: > When you pass `-fix` to clang-tidy, it will apply fix-its from the compiler > as well. See https://godbolt.org/z/7vzM4b and try adding a semicolon to the > end of the `switch`

[PATCH] D90348: [Driver] specify -stack-protector 0 for -fno-stack-protector

nickdesaulniers added a comment. Bumping for review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90348/new/ https://reviews.llvm.org/D90348 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D90194: [Driver] split LangOptions::SSPOff into SSPOFF and SSPUnspecified

nickdesaulniers added a comment. Bumping for review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90194/new/ https://reviews.llvm.org/D90194 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D90194: [Driver] split LangOptions::SSPOff into SSPOFF and SSPUnspecified

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:60 enum GCMode { NonGC, GCOnly, HybridGC }; - enum StackProtectorMode { SSPOff, SSPOn, SSPStrong, SSPReq }; + enum StackProtectorMode { SSPOff, SSPOn, SSPStrong, SSPReq, SSPUnspecified

[PATCH] D90194: [Driver] split LangOptions::SSPOff into SSPOFF and SSPUnspecified

nickdesaulniers added a comment. In D90194#2377337 , @rnk wrote: > Given that only three tests fail when the `nossp` attribute gets added from > `-cc1` with no stack protector, I think it's reasonable to add it and skip > adding the extra enum. Sorry, w

[PATCH] D90896: [Clang][Driver] default to nossp

nickdesaulniers created this revision. nickdesaulniers added reviewers: void, rnk. Herald added subscribers: cfe-commits, jdoerfert, atanasyan, jrtc27, kbarton, nemanjai. Herald added a project: clang. nickdesaulniers requested review of this revision. This patch does 2 things: 1. the default va

[PATCH] D90194: [Driver] split LangOptions::SSPOff into SSPOFF and SSPUnspecified

nickdesaulniers added a comment. D90896 is the alternative solution that doesn't add any new enumeration values. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90194/new/ https://reviews.llvm.org/D90194 __

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

nickdesaulniers added a comment. In D90180#2375878 , @aaron.ballman wrote: > In D90180#2374839 , @nickdesaulniers > wrote: > >> In D90180#2357247 , @aaron.ballman >> wrote

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

nickdesaulniers added inline comments. Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3 +; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s +; RUN: opt -always-inline -o - -S %s | FileCheck %s + aeubanks wrote: > This test fails with the NPM, > `opt

[PATCH] D87956: [IR] add fn attr for no_stack_protector; prevent inlining on mismatch

nickdesaulniers added inline comments. Comment at: llvm/test/Transforms/Inline/inline_nossp.ll:3 +; RUN: opt -passes='cgscc(inline)' %s -S | FileCheck %s +; RUN: opt -always-inline -o - -S %s | FileCheck %s + aeubanks wrote: > nickdesaulniers wrote: > > aeubanks

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