[PATCH] D138337: Add support for kcfi-seal optimization with LTO

nickdesaulniers added a comment. Is https://reviews.llvm.org/D142163 a blocker for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-commits mailing

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

nickdesaulniers added a comment. The test is for x86, does this also work for aarch64 with BTI? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-commits

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

nickdesaulniers added a comment. > If you're referring to ibt-seal, I assume a similar optimization could be > added for BTI too. Yes, please. Does it make sense to add to this patch, or a follow up on top? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D138337: Add support for kcfi-seal optimization with LTO

nickdesaulniers added a comment. ah, sorry, I think I might be conflating "kcfi-seal" with "ibt-seal?" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138337/new/ https://reviews.llvm.org/D138337 ___ cfe-c

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added a comment. Thanks for the patch! In D142609#4082259 , @xgupta wrote: > WIP && TODO- Fixed test > > Failed Tests (7): > > Clang :: C/drs/dr4xx.c > Clang :: Modules/explicit-build-extra-files.cpp > Clang :: Parser/cxx2a-concept-

[PATCH] D142675: [clang][CGDebugInfo] emit DW_LANG_C_plus_plus_{20|17} DW_LANG_C17

nickdesaulniers created this revision. nickdesaulniers added reviewers: dblaikie, jcranmer-intel. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Looks like these DW_AT_language tags made it into

[PATCH] D142675: [clang][CGDebugInfo] emit DW_LANG_C_plus_plus_{20|17} DW_LANG_C17

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. In D142675#4084279 , @dblaikie wrote: > This was discussed in D138597 & we decided > not to implement these due to the risk of breaking existing

[PATCH] D142675: [clang][CGDebugInfo] emit DW_LANG_C_plus_plus_{20|17} DW_LANG_C17

nickdesaulniers added a comment. Also noting that GCC 12.2.0 does not produce these newer `DW_AT_language` values when using -std={c17|c++17|c++20}`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142675/new/ https://reviews.llvm.org/D142675 _

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers added a comment. In D141451#4064298 , @dblaikie wrote: > Right - I was thinking more, as above, about directly using the existing > metadata generation (if it's too expensive to enable by default, then > possibly under an off-by-default

[PATCH] D143205: [clang] add __has_extension(gnu_asm_goto_with_outputs_full)

nickdesaulniers created this revision. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also move the line about __has_extension(gnu_asm_goto_with_outputs) so that it is more generally about asm g

[PATCH] D136497: [Clang] support for outputs along indirect edges of asm goto

nickdesaulniers marked 2 inline comments as done. nickdesaulniers added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1584 +Outputs may be used along any branches from the ``asm goto`` whether the +branches are taken or not. void wrote: > efrie

[PATCH] D143205: [clang] add __has_extension(gnu_asm_goto_with_outputs_full)

nickdesaulniers added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1588 + +Prior to clang-16, the output may only be used safely when the indirect +branches are not taken. Query for this difference with err...clang-17 Repository: rG LLVM Gi

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

nickdesaulniers added a comment. In D143300#4104469 , @MaskRay wrote: > `clang/test/CodeGen/init-randomized-struct-fwd-decl.c` passes without this > patch. Is it correct? Perhaps related to my comment about RecordDecl vs EnumDecl... or the tests might

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13583 +// bitwise one. +void Sema::diagnoseLogicalInsteadOfBitwise(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, Instead of `LHS` an

[PATCH] D137113: [Clang] refactor CodeGenFunction::EmitAsmStmt NFC

nickdesaulniers updated this revision to Diff 495174. nickdesaulniers added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137113/new/ https://reviews.llvm.org/D137113 Files: clang/lib/CodeGen/CGStmt.cpp Index: clang/lib/Code

[PATCH] D136497: [Clang] support for outputs along indirect edges of asm goto

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

[PATCH] D140508: [clang] fix -Wuninitialized for asm goto outputs on indirect edges.

nickdesaulniers updated this revision to Diff 495176. nickdesaulniers added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140508/new/ https://reviews.llvm.org/D140508 Files: clang/lib/Analysis/UninitializedValues.cpp clang/

[PATCH] D143205: [clang] add __has_extension(gnu_asm_goto_with_outputs_full)

nickdesaulniers updated this revision to Diff 495178. nickdesaulniers added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143205/new/ https://reviews.llvm.org/D143205 Files: clang/docs/LanguageExtensions.rst clang/include/c

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:18891 +// incomplete definition. +if (const auto *RD = dyn_cast(D)) + return !RD->isCompleteDefinition(); void wrote: > nickdesaulniers wrote: > > what about `

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:18891 +// incomplete definition. +if (const auto *RD = dyn_cast(D)) + return !RD->isCompleteDefinition(); nickdesaulniers wrote: > void wrote: > > nickdesaulni

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

nickdesaulniers added inline comments. Comment at: clang/test/CodeGen/init-randomized-struct-fwd-decl.c:2 +// RUN: %clang_cc1 -triple=x86_64-unknown-linux -emit-llvm -frandomize-layout-seed=1234567890abcdef < %s | FileCheck %s +// RUN: %clang_cc1 -triple=x86_64-unknown-linux -em

[PATCH] D143300: [randstruct] Don't allow implicit forward decl to stop struct randomization

nickdesaulniers accepted this revision. nickdesaulniers added a comment. Thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143300/new/ https://reviews.llvm.org/D143300 ___ cfe-commits m

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers updated this revision to Diff 488315. nickdesaulniers added a comment. - use std::string, demangle C++ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://reviews.llvm.org/D141451 Files: clang/include/clang/Basic/Di

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers updated this revision to Diff 488355. nickdesaulniers added a comment. - update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://reviews.llvm.org/D141451 Files: clang/include/clang/Basic/DiagnosticFrontendK

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers updated this revision to Diff 488356. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - remove test comment in commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://re

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers updated this revision to Diff 488360. nickdesaulniers added a comment. - add tests for alwaysinline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://reviews.llvm.org/D141451 Files: clang/include/clang/Basic/Diagn

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers updated this revision to Diff 488368. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. - replace inlined-from w/ inlined.from Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://review

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers updated this revision to Diff 488371. nickdesaulniers added a comment. - mention inlined.from in LangRef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://reviews.llvm.org/D141451 Files: clang/include/clang/Basic/

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:2467-2468 + +const Function *Callee = CI->getCalledFunction(); +if (Callee && (Callee->hasFnAttribute("dontcall-error") || + Callee->hasFnAttribut

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers added a comment. In D141451#4045414 , @efriedma wrote: > There are two different approaches we use for getting source locations for > diagnostics coming out of the backend; either embedding clang SourceLocations > into IR, or trying to t

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers added a comment. It would be really nice if we could limit this to a specific function somehow. Quick feedback from giving Diff 488727 a spin on the Linux kernel: via `ARCH=arm make LLVM=1 -j128 -s allyesconfig all` I found: CC drivers/net/ethernet/mellanox/mlx5/core/en

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers added inline comments. Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:44 + +const char *PassName = "stack-frame-layout"; + Consider replacing uses of `PassName` with `DEBUG_TYPE` since they have the same value. C

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers added inline comments. Comment at: clang/test/Frontend/stack-layout-remark.c:8 +// RUN: mkdir -p %t +// RUN: %clang_cc1 %s -emit-codegen-only -triple x86_64-unknown-linux-gnu -target-cpu corei7 -Rpass-analysis=stack-frame-layout -o /dev/null -O0 2>&1 | FileChe

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers accepted this revision. nickdesaulniers added a comment. In D135488#4049035 , @paulkirth wrote: > Actually if we add > > if (!isFunctionInPrintList(MF.getName())) >return false; > > we can filter by name Does name mangling compl

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers added a comment. Sorry, mind adding the documentation, too? Comment at: clang/test/Frontend/stack-layout-remark.c:8 +// RUN: mkdir -p %t +// RUN: %clang_cc1 %s -emit-codegen-only -triple x86_64-unknown-linux-gnu -target-cpu corei7 -Rpass-analysis=stack-frame-la

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers added a comment. In D135488#4049075 , @thegameg wrote: > You can easily... I'll just note that v1 of this patch IIRC was a note on the single individual instance of the `-Wframe-larger-than=` diagnostic. No additional flags for optimiz

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

nickdesaulniers added a comment. @paulkirth please don't forget to update the commit description with that flag so that I don't have to read the contents of the commit to find this flag again. If you're using `arc diff` to upload the patch, the `--verbatim` flag will update the phab descriptio

[PATCH] D137113: [Clang] refactor CodeGenFunction::EmitAsmStmt NFC

nickdesaulniers updated this revision to Diff 490245. nickdesaulniers added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137113/new/ https://reviews.llvm.org/D137113 Files: clang/lib/CodeGen/CGStmt.cpp Index: clang/lib/Code

[PATCH] D136497: [Clang] support for outputs along indirect edges of asm goto

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

[PATCH] D140508: [clang] fix -Wuninitialized for asm goto outputs on indirect edges.

nickdesaulniers updated this revision to Diff 490247. nickdesaulniers added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140508/new/ https://reviews.llvm.org/D140508 Files: clang/lib/Analysis/UninitializedValues.cpp clang/

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers added a comment. In D141451#4045658 , @efriedma wrote: > clang has a "LocTrackingOnly" setting for debug info, which emits DILocation > info into the IR, but emits a marker into the DICompileUnit to skip emitting > the .debug_info in the

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers added a comment. In D141451#4063335 , @dblaikie wrote: > In D141451#4063151 , > @nickdesaulniers wrote: > >> In D141451#4045658 , @efriedma >> wrote: >>

[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

nickdesaulniers added a comment. In D141451#4063519 , @dblaikie wrote: > In D141451#4063504 , > @nickdesaulniers wrote: > >> In D141451#4063335 , @dblaikie >> wrote: >>

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

nickdesaulniers added a comment. If the GCC developers split this into two distinct flags, should we land something we're just going to have to turn around and modify to match the new flags/semantics they've created? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126864/new/ https://re

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

nickdesaulniers added a comment. Diff 431607 boots for me in QEMU (triple checked that CONFIG_CC_HAS_SLS=y AND CONFIG_SLS=y were set this time ;) ). Comment at: clang/docs/ReleaseNotes.rst:371 +- Support ``-mharden-sls=all`` for X86. + pengfei wrote: > nickd

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

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Nice work Phoebe; thank you very much for this patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126137/new/ https://review

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

nickdesaulniers added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:368 -Select straight-line speculation hardening scope +Select straight-line speculation hardening scope (AArch64/X86 only). must be 'all', 'none', 'retbr'(AArch64), 'blr'(AArch64), 're

[PATCH] D126137: [X86] Add support for `-mharden-sls=[none|all|return|indirect-jmp]`

nickdesaulniers requested changes to this revision. nickdesaulniers added a subscriber: aaron.ballman. nickdesaulniers added a comment. This revision now requires changes to proceed. Let's get this rebased on top of D126511 . Comment at: clang/

[PATCH] D126137: [X86] Add support for `-mharden-sls=[none|all|return|indirect-jmp]`

nickdesaulniers accepted this revision. nickdesaulniers added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jsji. Thanks again Phoebe for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126137/new/

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

nickdesaulniers added a comment. In D126864#3564519 , @efriedma wrote: >> As for SOCK_MAXADDRLEN, that's a horrid hack, and the definition of struct >> sockaddr needs to change. :) > > Do we want some builtin define so headers can check if we're in > -f

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

nickdesaulniers added a comment. (Sorry for the wall of text) In D74094#4646297 , @tellenbach wrote: > No real comment on the issue itself but on the example as a former Eigen > maintainer (sorry for the noise if that's all obvious for you): Woah! Deus

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

nickdesaulniers added a comment. In D74094#4648319 , @rjmccall wrote: >> That's better than the status quo (no lifetime markers at all) but still >> suboptimal, and I don't think it will solve the particular case I care about >> (a particular Linux kerne

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

nickdesaulniers updated this revision to Diff 557119. nickdesaulniers added a comment. - undo ReturnsReference checks, add -Xclang -sloppy-temporary-lifetimes, add test for that, add release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74094/

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

nickdesaulniers updated this revision to Diff 557121. nickdesaulniers added a comment. - re-add sentence I accidentally dropped from the release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74094/new/ https://reviews.llvm.org/D74094 Files:

[PATCH] D74094: Reapply: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

nickdesaulniers updated this revision to Diff 557122. nickdesaulniers added a comment. - git clang-format HEAD~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74094/new/ https://reviews.llvm.org/D74094 Files: clang/docs/ReleaseNotes.rst clang/i

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers updated this revision to Diff 541178. nickdesaulniers added a comment. - fix typo in release doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155342/new/ https://reviews.llvm.org/D155342 Files: clang/docs/ReleaseNotes.rst clan

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:361 + if (!GS->isAsmGoto()) +break; // Remember both what scope a goto is in as well as the fact that we have rjmccall wrote: > You can pull the `GCCAsmStmtCla

[PATCH] D155506: [clang][JumpDiagnostics] use StmtClass rather than dyn_cast chain NFC

nickdesaulniers created this revision. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Avoid the anti-pattern of: if dyn_cast: ... elif dyn_cast: ... elif dyn_cast: ... And si

[PATCH] D155522: [clang][JumpDiagnostics] refactor helper

nickdesaulniers created this revision. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added subscribers: cfe-commits, wangpc. Herald added a project: clang. Posting to phab for separate review from https://reviews.llvm.org/D155342#4507805 Will squash this i

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:361 + if (!GS->isAsmGoto()) +break; // Remember both what scope a goto is in as well as the fact that we have rjmccall wrote: > nickdesaulniers wrote: > > rjmc

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:361 + if (!GS->isAsmGoto()) +break; // Remember both what scope a goto is in as well as the fact that we have nickdesaulniers wrote: > rjmccall wrote: > > nick

[PATCH] D155525: WIP

nickdesaulniers created this revision. Herald added a project: All. nickdesaulniers requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Alternate to D155522 based on https://reviews.llvm.org/D155342#4507805

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:361 + if (!GS->isAsmGoto()) +break; // Remember both what scope a goto is in as well as the fact that we have nickdesaulniers wrote: > nickdesaulniers wrote: >

[PATCH] D155506: [clang][JumpDiagnostics] use StmtClass rather than dyn_cast chain NFC

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. no worries then Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155506/new/ https://reviews.llvm.org/D155506 ___ cfe-commits mailin

[PATCH] D155525: WIP

nickdesaulniers abandoned this revision. nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:362 // it. This makes the second scan not have to walk the AST again. +RecordJumpScope: LabelAndGotoScopes[S] = ParentScope;

[PATCH] D155522: [clang][JumpDiagnostics] refactor helper

nickdesaulniers abandoned this revision. nickdesaulniers added a comment. prefer D155525 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155522/new/ https://reviews.llvm.org/D155522

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers updated this revision to Diff 541619. nickdesaulniers added a comment. - squash D155525 as per @rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155342/new/ https://reviews.llvm.org/D155342

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers marked 2 inline comments as done. nickdesaulniers added a comment. Thanks for the reviews and advice. Any parting thoughts @rjmccall ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155342/new/ https://reviews.llvm.org/D155342 _

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added a comment. In D142609#4507696 , @nathanchance wrote: > but I see a new one along a similar line as those: > > https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/v3d/v3d_drv.h#L343 I only see 4 instances of `NSEC_PER_S

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

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1858 +static bool isEmptyClass(VarDecl const &D) { + const Type *Ty = D.getType().getTypePtr(); inconsistent east const vs west const Comment at: clang/lib/CodeG

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers updated this revision to Diff 541639. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. - add comment as per @rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155342/new/ https://reviews.llvm.o

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:658 +if (auto *G = dyn_cast(Jump)) { + for (AddrLabelExpr *L : G->labels()) { rjmccall wrote: > I think it would be good to leave a comment here like this: > > > We

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:658 +if (auto *G = dyn_cast(Jump)) { + for (AddrLabelExpr *L : G->labels()) { rjmccall wrote: > nickdesaulniers wrote: > > rjmccall wrote: > > > I think it would be g

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers updated this revision to Diff 541705. nickdesaulniers added a comment. - rewrite comment based on feedback from @rjmccall and @efriedma Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155342/new/ https://reviews.llvm.org/D155342 File

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers added a comment. In D155342#4511879 , @rjmccall wrote: >> We need to check the scopes like we would for direct goto, because we don't >> want to bypass non-trivial destructors. > > I think this is the basic misunderstanding here. Direct

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8360 + // Do not constant fold an R-value. + if (Info.EvalMode == EvalInfo::EM_ConstantFold && !E->isLValue()) +return false; efriedma wrote: > Checking isLValue() doesn't mak

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8360 + // Do not constant fold an R-value. + if (Info.EvalMode == EvalInfo::EM_ConstantFold && !E->isLValue()) +return false; efriedma wrote: > nickdesaulniers wrote: > > efri

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers updated this revision to Diff 542081. nickdesaulniers marked 4 inline comments as done. nickdesaulniers added a comment. - add link to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110728 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8360 + // Do not constant fold an R-value. + if (Info.EvalMode == EvalInfo::EM_ConstantFold && !E->isLValue()) +return false; nickdesaulniers wrote: > efriedma wrote: > > nick

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers updated this revision to Diff 542125. nickdesaulniers added a comment. - add @efriedma's test case - check if the extending decl is not a reference, rather than for X or L values Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151587/n

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers updated this revision to Diff 542127. nickdesaulniers added a comment. - reformat Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151587/new/ https://reviews.llvm.org/D151587 Files: clang/lib/AST/ExprConstant.cpp clang/lib/CodeGe

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8360 + // Do not constant fold an R-value. + if (Info.EvalMode == EvalInfo::EM_ConstantFold && !E->isLValue()) +return false; efriedma wrote: > nickdesaulniers wrote: > > nick

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8360 + // Do not constant fold an R-value. + if (Info.EvalMode == EvalInfo::EM_ConstantFold && !E->isLValue()) +return false; efriedma wrote: > efriedma wrote: > > nickdesauln

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added a comment. In D151587#4516772 , @efriedma wrote: > This seems to work: > > diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp > index 25d3535e..98b1e4d 100644 > --- a/clang/lib/AST/Expr.cpp > +++ b/clang/lib/AST/Exp

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added a comment. In D151587#4519700 , @efriedma wrote: >> It seems like perhaps we want to call EvaluateAsLValue in >> Expr::isConstantInitializer AFTER checking the statement class. Otherwise >> we're basically reimplementing that big s

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers updated this revision to Diff 542594. nickdesaulniers added a comment. - reorder the check for IsForRef relative to the switch on statement class - add @efriedma's suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151587/new/

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers updated this revision to Diff 542600. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - add more context to the commit description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151587/new/ https

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers updated this revision to Diff 542603. nickdesaulniers added a comment. - refactor common tail from IsForRef check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151587/new/ https://reviews.llvm.org/D151587 Files: clang/lib/AST/Exp

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:3462-3468 ->isConstantInitializer(Ctx, false, Culprit); case CXXDefaultArgExprClass: return cast(this)->getExpr() ->isConstantInitializer(Ctx, false, Culprit); case CXXDefaultInitEx

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers updated this revision to Diff 542607. nickdesaulniers marked an inline comment as done. nickdesaulniers added a comment. - force ForRef to false for CK_LValueToRValue CastExprs, as per @efriedma Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D76096: [clang] allow const structs to be constant expressions for C

nickdesaulniers updated this revision to Diff 542627. nickdesaulniers retitled this revision from "[clang] allow const structs to be constant expressions in initializer lists" to "[clang] allow const structs to be constant expressions for C". nickdesaulniers edited the summary of this revision. n

[PATCH] D76096: [clang] allow const structs to be constant expressions for C

nickdesaulniers added inline comments. Comment at: clang/test/Sema/builtins.c:181-186 + ASSERT(!OPT(test17_c)); + ASSERT(!OPT(&test17_c[0])); + ASSERT(!OPT((char*)test17_c)); ASSERT(!OPT(test17_d));// expected-warning {{folding}} ASSERT(!OPT(&test17_d[0]));//

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/Expr.cpp:3462-3468 ->isConstantInitializer(Ctx, false, Culprit); case CXXDefaultArgExprClass: return cast(this)->getExpr() ->isConstantInitializer(Ctx, false, Culprit); case CXXDefaultInitEx

[PATCH] D76096: [clang] allow const structs to be constant expressions for C

nickdesaulniers planned changes to this revision. nickdesaulniers added inline comments. Comment at: clang/test/Sema/builtins.c:181-186 + ASSERT(!OPT(test17_c)); + ASSERT(!OPT(&test17_c[0])); + ASSERT(!OPT((char*)test17_c)); ASSERT(!OPT(test17_d));// expected-warnin

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

nickdesaulniers requested changes to this revision. nickdesaulniers added a comment. This revision now requires changes to proceed. Herald added a project: All. Folded into D151587 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:669 +if (JumpScope != TargetScope) + DiagnoseIndirectOrAsmJump(G, JumpScope, LD, TargetScope); + } shafik

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf023f5cdb2e6: [clang][JumpDiagnostics] ignore non-asm goto target scopes (authored by nickdesaulniers). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

nickdesaulniers added a comment. In D154696#4513510 , @cor3ntin wrote: > FYI, this can be relanded after https://reviews.llvm.org/D155342 is merged D155342 is all merged up Repository: rG LLVM Github Monorepo CHANG

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers planned changes to this revision. nickdesaulniers added a comment. Cool, looks like the libc++ presubmit tests have already spotted some unintended changes. Will root cause+fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151587/n

[PATCH] D151587: [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first

nickdesaulniers added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8384-8385 const MaterializeTemporaryExpr *E) { + if (Info.EvalMode == EvalInfo::EM_ConstantFold) +return false; + removing these two lines fixes the following regressions:

<    5   6   7   8   9   10   11   12   13   14   >