[PATCH] D112768: [ARM] implement support for TLS register based stack protector

nickdesaulniers added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3177-3179 + if (!Args.hasArg(options::OPT_mstack_protector_guard_offset_EQ)) { +D.Diag(diag::err_drv_ssp_missing_offset_argument) +<< A->getOption().getName() <<

[PATCH] D113026: [ARM] reject -mtp=cp15 if target subarch does not support it

nickdesaulniers added a comment. Thanks for the patch! Don't forget to run `git-clang-format HEAD~`. Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:160-161 +if (ThreadPointer == ReadTPMode::Cp15 && +getARMSubArchVersionNumber(Triple) < 7 && +

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

nickdesaulniers added a comment. Codegen looks good, probably just need an additional conditional on `ARMSubtarget::isReadTPHard()`. With that and some more tests for cases we don't intend to support (thumb[1], soft tp) this LGTM. Great job @ardb ! Comment at:

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers accepted this revision. nickdesaulniers added a comment. thanks again for all of the work that went into this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112059/new/ https://reviews.llvm.org/D112059 ___ cfe-commits mailing

[PATCH] D112761: cfi-icall: Add -fsanitize-cfi-promotion-aliases

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. please don't forget to run `git-clang-format HEAD~`. Thanks for the patch, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:250 void Thumb2InstrInfo::expandLoadStackGuard( MachineBasicBlock::iterator MI) const { what about `Thumb1InstrInfo::expandLoadStackGuard`? Do we have `mrc`

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

nickdesaulniers added a comment. Nice job! This looks like it hits every place that I was looking at for this. In terms of tests, https://reviews.llvm.org/D100919 and https://reviews.llvm.org/D102742 are probably interesting. In particular, we should test that clang no longer rejects

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

nickdesaulniers added inline comments. Comment at: clang/include/clang/AST/APValue.h:195 unsigned CallIndex, Version; + bool NoCFIValue : 1; }; Is there be padding off the end of the bitfield? Or does this actually change the

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

nickdesaulniers added inline comments. Comment at: clang/lib/AST/APValue.cpp:44-46 + false} {} APValue::LValueBase::LValueBase(const Expr *P, unsigned I, unsigned V) +: Ptr(P), Local{I, V, false} {}

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/builtin-addressof-nocfi.c:2 +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=cfi-icall -o - %s | FileCheck %s + +void a(void) {} mind adding a test case to this file

[PATCH] D112143: [X86][ABI] Do not return float/double from x87 registers when x87 is disabled

nickdesaulniers added inline comments. Comment at: clang/test/Driver/x86-target-features.c:5 // RUN: %clang -target i386-unknown-linux-gnu -march=i386 -mno-80387 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-X87 %s +// RUN: %clang -target i386-unknown-linux-gnu -march=i386

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added a comment. Here's a [hastily and poorly written] script to measure the average cycle counts for 30 invocations using linux `perf`: https://gist.github.com/nickdesaulniers/4a20ba10c26ac2ad02cb0425b8b0f826 For Diff 382671 (latest; storing redecl state), builds of the linux

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/arm-tphard.c:10 +} + peter.smith wrote: > nickdesaulniers wrote: > > ardb wrote: > > > nickdesaulniers wrote: > > > > Let's make this a test under llvm/test/CodeGen/, using IR: > > > > ``` > >

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd7e089f2d6a5: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed (authored by ardb, committed by nickdesaulniers). Repository:

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

nickdesaulniers added a comment. In D112600#3091959 , @ardb wrote: > Thanks all. Could someone with commit access please merge this? Thanks. Sure thing; I'll pick it up. Thanks again for the patch! CHANGES SINCE LAST ACTION

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:96 +static cl::opt SkipRaxSetup( +"x86-skip-rax-setup", cl::init(false), MaskRay wrote: > nickdesaulniers wrote: > > If it's a command line option rather than

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Cool, if you wouldn't mind additional extending this patch to cover the intrinsic that the front end also generates, this LGTM: diff --git

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/arm-tphard.c:10 +} + ardb wrote: > nickdesaulniers wrote: > > Let's make this a test under llvm/test/CodeGen/, using IR: > > ``` > > ; RUN: llc --mtriple=armv7-linux-gnueabihf -o - %s |

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. Thanks for the patch; I'd like to see this encoded in the IR (perhaps as a function level attribute), the `-mno-` variant added, and perhaps help the user if they

[PATCH] D112600: [ARM] Use hardware TLS register in Thumb2 mode when -mtp=cp15 is passed

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. Thanks for the patch! Comment at: clang/test/CodeGen/arm-tphard.c:10 +} + Let's make this a test under llvm/test/CodeGen/, using

[PATCH] D112415: [Driver] ignore -maccumulate_outgoing_args option

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. Let me check on https://bugs.llvm.org/show_bug.cgi?id=28145 before we start ignoring this. That might be the right approach, but let me check. Otherwise we might

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

nickdesaulniers added a comment. In D112413#3089073 , @nickdesaulniers wrote: > Fixes https://bugs.llvm.org/show_bug.cgi?id=23258? sorry, I missed the PR # in the description; my mistake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D112143: [X86][ABI] Do not return float/double from x87 registers when x87 is disabled

nickdesaulniers added a comment. In D112143#3089072 , @nickdesaulniers wrote: > Fixes https://bugs.llvm.org/show_bug.cgi?id=51498? sorry, I missed the PR # in the description; my mistake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

nickdesaulniers added a comment. Fixes https://bugs.llvm.org/show_bug.cgi?id=23258? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112413/new/ https://reviews.llvm.org/D112413 ___ cfe-commits mailing

[PATCH] D112143: [X86][ABI] Do not return float/double from x87 registers when x87 is disabled

nickdesaulniers added a comment. Fixes https://bugs.llvm.org/show_bug.cgi?id=51498? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112143/new/ https://reviews.llvm.org/D112143 ___ cfe-commits mailing

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/user-func-gnu-inline-redecl.c:20 + return some_size(s); +} this test passes before this patch is applied; I wonder if we have existing coverage in tree for this case? Surprisingly, I don't

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added a comment. Actually, it looks like: diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 69d2ef631872..8e77cdef2ed5 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -10927,6 +10927,10 @@ bool

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Thank you for fixing this terrible edge case; LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112059/new/ https://reviews.llvm.org/D112059 ___ cfe-commits mailing

[PATCH] D100756: [llvm-rc] [4/4] Add a GNU windres-like frontend to llvm-rc

nickdesaulniers added a comment. thanks for this patch @mstorsjo ! This is help AOSP LLVM move our windows builds of LLVM away from MinGW to LLVM! https://android-review.googlesource.com/c/toolchain/llvm_android/+/1867380/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302 // in case the function pointer is referenced somewhere. - if (FD->isInlineBuiltinDeclaration() && Fn) { + if (Fn) { std::string FDInlineName = (Fn->getName() +

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302 // in case the function pointer is referenced somewhere. - if (FD->isInlineBuiltinDeclaration() && Fn) { + if (Fn) { std::string FDInlineName = (Fn->getName() +

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302 // in case the function pointer is referenced somewhere. - if (FD->isInlineBuiltinDeclaration() && Fn) { + if (Fn) { std::string FDInlineName = (Fn->getName() +

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added a comment. Bumping for an update here. We can tolerate a build breakage for our older kernels over the weekend, but we should really try to get this resolved by EOW, otherwise we need to look into reverting: - 3d6f49a56995b845c40be5827ded5d1e3f692cec

[PATCH] D112135: [ARM] Fix inline assembly referencing floating point registers on soft-float targets

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/D112135/new/ https://reviews.llvm.org/D112135

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added subscribers: aaron.ballman, rsmith. nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1302 // in case the function pointer is referenced somewhere. - if (FD->isInlineBuiltinDeclaration() && Fn) { + if (Fn) {

[PATCH] D112059: Fix inline builtin handling in case of redefinition

nickdesaulniers added a subscriber: nathanchance. nickdesaulniers added a comment. Yes; GCC does behave this way. It does not consider a non-gnu-inline redefinition an error, and it does seem to prefer the non-gnu-inline redeclaration when both are present, AFAICT. The test is verifying that

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. Also reported here: https://lore.kernel.org/llvm/1817774243.12566.1634255267713@localhost/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111009/new/ https://reviews.llvm.org/D111009

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. See also: https://github.com/ClangBuiltLinux/linux/issues/1477. Surprising that this didn't show up in newer kernels, just older (but still supported) kernel versions. Sorry I missed that in my tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D76169: [WIP][AST] Allow ExprConstant to evaluate structs in C.

nickdesaulniers added a subscriber: erichkeane. nickdesaulniers added a comment. Looks like @erichkeane was in the neighborhood in 606a734755d1fb6c35a17680d0c251f834b79334 ("[PR47636] Fix tryEmitPrivate to handle

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers added a comment. In D107613#3040402 , @dschuff wrote: > It looks like this error is intended to catch mismatches for attributes that > can affect codegen such as noreturn (in which case it makes sense to have it > as an error) but it

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers accepted this revision. nickdesaulniers added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGExpr.cpp:4901 + llvm::Function *Fn = llvm::cast(CalleePtr); + llvm::Module = CGF.CGM.getModule(); +

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. Diff 376794 builds! Let's still add a test for inline __attribute__((__always_inline__)) __attribute__((gnu_inline)) void* memcpy() {} though. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111009/new/ https://reviews.llvm.org/D111009

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. With Diff 376790 inline __attribute__((__always_inline__)) __attribute__((gnu_inline)) void* memcpy() {} `clang -O2` is producing undefined references to `memcpy.inline`. Please add that to the new unit tests added here. diff --git

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. With Diff 376790, I'm seeing a similar failure to https://github.com/ClangBuiltLinux/linux/issues/1466#issue-1011472964, but from different TUs. ld.lld: error: duplicate symbol: memcpy.inline >>> defined at file.c >>>

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. Here's a reduced test case from the kernel. Let's add a unit test for it. const void *con_unify_unimap_p1; extern inline __attribute__((__always_inline__)) __attribute__((gnu_inline)) int memcmp(const void *p, const void *q, unsigned long size) {

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4911 + if (!Clone) { +Clone = llvm::Function::Create(Fn->getFunctionType(), Fn->getLinkage(), + Fn->getAddressSpace(), given the

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. Note: kernel tests need to have `CONFIG_FORTIFY_SOURCE=y` enabled in the config to use the fortified routines; this isn't enabled by default in an x86_64 `defconfig` build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D111009: Update inline builtin handling to honor gnu inline attribute

nickdesaulniers added a comment. hmm...this seems to cause a bunch of ICEs building the kernel: Global is external, but doesn't have external or weak linkage! i64 (i8*)* @strlen fatal error: error in backend: Broken module found, compilation aborted! Global is external, but doesn't have

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. We'll probably need to investigate code gen a little. A mainline linux kernel defconfig built with `CONFIG_ZERO_CALL_USED_REGS=y` enabled doesn't boot, for

[PATCH] D110869: [X86] Implement -fzero-call-used-regs option

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6108 + let Content = [{ +This attribute, when attached to a funcditon, causes the compiler to zero a +subset of all call-used registers before the function returns. It's used to

[PATCH] D110065: [AArch64] Add support for the 'R' architecture profile.

nickdesaulniers added a comment. In D110065#3033050 , @labrinea wrote: > That said my current approach will be breaking the current tools behavior: > when the user only specifies the triple and not -march then they will be > targeting the intersection,

[PATCH] D110364: [clang] Rework dontcall attributes

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. thanks for the patch! Comment at: llvm/lib/IR/DiagnosticInfo.cpp:408-427 + if (F->hasFnAttribute("dontcall-error")) { +unsigned LocCookie = 0; +

[PATCH] D110379: [Driver] Remove confusing *-linux-android detection with non-android --target=

nickdesaulniers added a comment. Sorry, I missed the email for this code review, let me check my filters aren't too agressive. > The Android kernel build does not use the Android triple. That's no longer the case, so we might as well just wholesale revert D53463

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. In D106030#3018754 , @aeubanks wrote: > Would it be ok to split this into two attributes, dontcall-warn and > dontcall-error? So we rely less on the Clang AST when determining if a > dontcall call should be emitted as a

[PATCH] D109967: Simplify handling of builtin with inline redefinition

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Looks reasonable. Can you give us some time to test this on the Linux kernel? Comment at: clang/test/CodeGen/memcpy-inline-builtin.c:3 + +// RUN:

[PATCH] D109967: Simplify handling of builtin with inline redefinition

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/memcpy-inline-builtin.c:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s +// nickdesaulniers wrote: > Would `-emit-codegen-only`

[PATCH] D109967: Simplify handling of builtin with inline redefinition

nickdesaulniers added a comment. Please amend the bug description to link to: - https://bugs.llvm.org/show_bug.cgi?id=50322 - https://lore.kernel.org/lkml/20210822075122.864511-17-keesc...@chromium.org/ (You can do `git commit --amend; arc diff --verbatim` to have `arc` update the patch

[PATCH] D109967: Simplify handling of builtin with inline redefinition

nickdesaulniers added a comment. Thanks for the patch! Please fix the lint checks. `git-clang-format HEAD~` should help, and IIRC there is a git hook when using `arc diff` (though maybe that requires one time setup? I seem to have an `.arclint` file in my `llvm-projects` checkout.

[PATCH] D107420: [sema] Disallow __builtin_mul_overflow under special condition.

nickdesaulniers added a comment. In D107420#2929115 , @craig.topper wrote: > In D107420#2929039 , @aaron.ballman > wrote: > >> In D107420#2928975 , @craig.topper >>

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1890 -if (auto FD = dyn_cast(D)) - return CGM.GetAddrOfFunction(FD); +if (auto FD = dyn_cast(D)) { + auto *C = CGM.GetAddrOfFunction(FD); fix linter

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. Fixup: https://reviews.llvm.org/D108718 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 ___ cfe-commits mailing list

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. I'm getting test failures for the newly added llvm/test/CodeGen/X86/attr-dontcall.ll, thought there's not much info other than > Exit Code: 1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/

[PATCH] D106030: [Clang] add support for error+warning fn attrs

This revision was automatically updated to reflect the committed changes. Closed by commit rG846e562dcc6a: [Clang] add support for error+warning fn attrs (authored by nickdesaulniers). Changed prior to commit: https://reviews.llvm.org/D106030?vs=366717=368678#toc Repository: rG LLVM Github

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'error' and 'warning' attributes are not compatible}} +__attribute__((warning("foo"))) int bad5(void); // expected-note

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366717. nickdesaulniers added a comment. - reorder diag vs note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 Files: clang/docs/ReleaseNotes.rst

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'error' and 'warning' attributes are not compatible}} +__attribute__((warning("foo"))) int bad5(void); // expected-note

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366356. nickdesaulniers added a comment. - remove unnecessary parens Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 Files: clang/docs/ReleaseNotes.rst

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:3368 + Diag(Old->getLocation(), diag::note_previous_declaration); + New->dropAttr(); +} Should be dropping `ErrorAttr`. Comment at:

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366355. nickdesaulniers marked 4 inline comments as done. nickdesaulniers added a comment. - fix tests on windows, let diag eng format decl, drop correct attr, refactor to handle merging as InheritableAttr Repository: rG LLVM Github Monorepo

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

nickdesaulniers added inline comments. Comment at: clang/test/SemaCXX/switch-implicit-fallthrough.cpp:211-214 +case 224: + n += 400; +case 225: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:964-965 +bool match = +(EA->isError() && NewAttrSpellingIndex < ErrorAttr::GNU_warning) || +(EA->isWarning() && NewAttrSpellingIndex >= ErrorAttr::GNU_warning); +if

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366050. nickdesaulniers marked 4 inline comments as done. nickdesaulniers added a comment. - combine c++ w/ c test - use -verify= rather than -D - remove REQUIRES - go back to string comparison Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11172 +def err_attribute_removed_on_redeclaration : Error< + "'%0' attribute removed from redeclared variable">; aaron.ballman wrote: > This diagnostic is a

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 364892. nickdesaulniers marked 8 inline comments as done. nickdesaulniers added a comment. - rebase on D107613 , reuse diag::err_attribute_missing_on_first_decl - test matching redeclarations - remove getSpelling

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

This revision was automatically updated to reflect the committed changes. Closed by commit rGd238b6028582: [Clang][DiagnosticSemaKinds] combine diagnostic texts (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107613/new/

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:3686 +Diag(Old->getLocation(), diag::note_previous_declaration); + } should `CXX11NoReturnAttr` be `dropAttr` on `New` like the other cases, too? Repository: rG

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers updated this revision to Diff 364858. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. - more use of note_previous_declaration, fix WeakImportAttr, too Repository: rG LLVM Github Monorepo

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers planned changes to this revision. nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:3685 +<< NRA; +notePreviousDefinition(Old, New->getLocation()); + } aaron.ballman wrote: > nickdesaulniers

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:3685 +<< NRA; +notePreviousDefinition(Old, New->getLocation()); + } ah, this isn't quite right. It replaces warning of the first declaration with warning of

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers updated this revision to Diff 364838. nickdesaulniers marked 2 inline comments as done. nickdesaulniers added a comment. - prefer getAttr to hasAttr+getAttr, remove another diag Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D107613: [Clang][DiagnosticSemaKinds] combine diagnostic texts

nickdesaulniers created this revision. nickdesaulniers added a reviewer: aaron.ballman. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The diagnostic texts for warning on attributes that don't appear on the initial

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1205 def BackendOptimizationFailure : DiagGroup<"pass-failed">; +def BackendUserDiagnostic : DiagGroup<"user-diagnostic">;

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 364280. nickdesaulniers marked 5 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - remove stale todos, change warning to -Wwarning-attributes to match GCC, update docs Repository: rG

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6057 +assertions that depend on optimizations, while providing diagnostics +pointing to precise locations of the call site in the source. + }]; @aaron.ballman

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers planned changes to this revision. nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3819 + let Args = [StringArgument<"UserDiagnostic">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation =

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 363888. nickdesaulniers marked 8 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. Herald added subscribers: ormris, steven_wu. - LTO test, diagnose inheritability Repository: rG LLVM

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; aaron.ballman wrote: > nickdesaulniers wrote: > > aaron.ballman wrote: > > > I think this should be an inheritable

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783 + else +Diags.Report(DiagID) << CalleeName << D.getMsgStr(); +} nickdesaulniers wrote: > aaron.ballman wrote: > > nickdesaulniers wrote: > > > aaron.ballman wrote: >

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6026 +depend on optimizations, while providing diagnostics pointing to precise +locations of the call site in the source. + }]; aaron.ballman wrote: >

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 363581. nickdesaulniers marked 2 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - reusing existing Diag/Notes, remove test comment, add test for indirect call Repository: rG LLVM

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:45 + } + return true; +} samitolvanen wrote: > nickdesaulniers wrote: > > samitolvanen wrote: > > > nickdesaulniers wrote: > > > > Can llvm::any_of or

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:45 + } + return true; +} samitolvanen wrote: > nickdesaulniers wrote: > > Can llvm::any_of or llvm::none_of be used here? > > llvm/ADT/STLExtras.h > Maybe, but

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:39 + // Promotion aliases are used only in inline assembly. It's safe to + // simply skip unusual names. Matches MCAsmInfo::isAcceptableChar(). + for (const char : Name) {

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/test/Frontend/backend-attribute-error-warning.c:29 + duplicate_errors(); // expected-error {{call to 'duplicate_errors' declared with attribute error: two}} + // TODO: why is this a warning not an error +

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3823 + +def Warning : Attr { + let Spellings = [GCC<"warning">]; aaron.ballman wrote: > nickdesaulniers wrote: > > aaron.ballman wrote: > > > Given that the only functional

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 360324. nickdesaulniers marked 4 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - merge ErrorAttr with WarningAttr - fix dumb short circuit bug - rename diag group - handle multiple

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:77 + // references from inline assembly. + std::string Alias = ".set \"" + OldName + "\",\"" + NewName + "\"\n"; + ExportM.appendModuleInlineAsm(Alias);

[PATCH] D104058: ThinLTO: Fix inline assembly references to static functions with CFI

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:77 + // references from inline assembly. + std::string Alias = ".set \"" + OldName + "\",\"" + NewName + "\"\n"; + ExportM.appendModuleInlineAsm(Alias);

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; aaron.ballman wrote: > I think this should be an inheritable attribute (same below) so that > redeclarations get

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 359912. nickdesaulniers marked 5 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. Herald added a subscriber: pengfei. - change IR Attr to dontcall - check during ISel(s) - rename td diag -

<    3   4   5   6   7   8   9   10   11   12   >