[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth created this revision. paulkirth added reviewers: phosek, leonardchan, jakehehrlich, mcgrathr. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar, mgorny. Herald added a project: clang. **Overview**: This patch contains a prototype of the basic functionality of clang-misex

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 211831. paulkirth added a comment. Refactors some debug code to be more centralized and cleans up some comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65300/new/ https://reviews.llvm.org/D65300 Files: clang/include/clang/Basic/CodeGenOp

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth added a comment. In D65300#1601733 , @xbolva00 wrote: > Nice work! Glad to hear you like it. > It would be nice if we also have “-fsuggest-expect” so we could fix perf > issues thanks to PGO counters even for non-PGO builds. What do you think

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 211853. paulkirth added a comment. Add missing test for switch statements when the expected value is not a compile time constant. Make sure that when the expected value cannot be evaluated, we do not issue any warnings or errors Repository: rG LLVM Gi

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 212492. paulkirth added a comment. Update diff to have proper context on Phabricator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65300/new/ https://reviews.llvm.org/D65300 Files: clang/include/clang/Basi

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 213059. paulkirth marked 2 inline comments as done. paulkirth added a comment. Fix typo in comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65300/new/ https://reviews.llvm.org/D65300 Files: clang/inc

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth marked 2 inline comments as done. paulkirth added inline comments. Comment at: clang/lib/CodeGen/MisExpect.cpp:82 + if (ExpectedTrueBranch) { +const llvm::BranchProbability HotFunctionThreshold(1, 100); +Scaled = HotFunctionThreshold.scale(CurrProfCount); -

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 213164. paulkirth added a comment. Address feedback from review 1. Fixed comments in License headers 2. removed excess {} 3. Changed conditional assignment to ternary operation 4. Moved local functions into anonymous namespace 5. Added reference to the orig

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 213167. paulkirth added a comment. Update documentation and fix typos Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65300/new/ https://reviews.llvm.org/D65300 Files: clang/include/clang/Basic/CodeGenOption

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 213379. paulkirth edited the summary of this revision. paulkirth added a comment. Update threshold values to match those assigned when lowering __builtin_expect intrinsic. I've modified the branch probability to match the probability assigned in LowerExpe

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth updated this revision to Diff 213725. paulkirth added a comment. Use existing LLVM code for mapping case literals to their case arms. This update refactors a great deal of the implementation and test code. 1. Removes the CaseMap data structure completely. LLVM already maintains a mapp

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

paulkirth created this revision. paulkirth added reviewers: phosek, leonardchan, jakehehrlich, mcgrathr. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mgorny. Herald added projects: clang, LLVM. paulkirth added a parent revision: D65300: [clang] [CodeGen] clang-misexpect prototyp

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

paulkirth added a comment. In D66324#1632653 , @lebedev.ri wrote: > This is marked as child revision of D65300 > but it seems like they both add > the same logic, just into different components, D65300 >

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

paulkirth updated this revision to Diff 215643. paulkirth edited the summary of this revision. paulkirth added a comment. Herald added subscribers: dexonsmith, steven_wu, mehdi_amini. Removes standalone clang-misexpect from revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

paulkirth updated this revision to Diff 215646. paulkirth added a comment. Remove reference to clang-misexpect from CMakeLists.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clan

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

paulkirth added a comment. > So it is possible to handle this completely in the backed, but the diagnostic > output is not fantastic when using clang based instrumentation. In > particular, we would need to emit the diagnostic in LowerExpectIntrisic.cpp > by checking if the branch weight meta

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

paulkirth updated this revision to Diff 215734. paulkirth added a comment. Remove frontend components of clang-misexpect in favor of backend implementations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324

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

paulkirth updated this revision to Diff 215938. paulkirth edited the summary of this revision. paulkirth added a comment. - Update CodeGenOptions to remove -fmisexpect - Access the LLVMContext directly - Add -line-tables-only for more accurate diagnostics Fixes various issues with tests and clean

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

paulkirth updated this revision to Diff 215940. paulkirth added a comment. Fix missing context in prior diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/include/clang/Basi

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

paulkirth updated this revision to Diff 219617. paulkirth added a comment. Add a new profdata file to use w/ misexpect-nonconst-switch.c ASAN exposed an issue where a function may hash the same even if the number of counters is different. This means that when a PGO profile is read in, it is pos

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

paulkirth marked an inline comment as done. paulkirth added inline comments. Comment at: compiler-rt/trunk/lib/profile/xxhash.h:41-42 + +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringRef.h" + chandlerc wrote: > Sorry folks, but you can't do this. > >

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

paulkirth added a comment. In D66324#1665784 , @lebedev.ri wrote: > In D66324#1665773 , @gribozavr wrote: > > > Reverted in r371598. > > > > Another concern that I have is cross-compilation. LLVM's ADT is not set up

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

paulkirth updated this revision to Diff 219728. paulkirth added a comment. Revert mismerge when landing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/include/clang/Basic/Di

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

paulkirth updated this revision to Diff 220239. paulkirth added a comment. Addresses problems running the standalone tool w/ the libTooling executors. When using the CodeGenAction and setting LLVM backend options, I found several places where data races occurred. This seems like a more significa

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

paulkirth added a comment. In D67253#1670569 , @lebedev.ri wrote: > Layering feels weird here. > Can this be implemented as/limited to just a > `run-clang-misexpect.py` wrapper over clang itself? > That would be best IMHO. I discussed the concurrency

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

paulkirth added a comment. In D67253#1670681 , @lebedev.ri wrote: > Re concurrency - you had standalone `LLVMContext` for each thread, right? I believe there was, but this is just whatever happens when libtooling creates and executes a compiler invocati

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

paulkirth added inline comments. Comment at: llvm/trunk/include/llvm/IR/DiagnosticInfo.h:79 + DK_FirstPluginKind, + DK_MisExpect }; jrtc27 wrote: > Is this not going to clash with the first caller to > `getNextAvailablePluginDiagnosticKind`? `DK_FirstPluginKi

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

paulkirth marked an inline comment as done. paulkirth added inline comments. Comment at: llvm/trunk/include/llvm/IR/DiagnosticInfo.h:79 + DK_FirstPluginKind, + DK_MisExpect }; paulkirth wrote: > jrtc27 wrote: > > Is this not going to clash with the first calle

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. In D131306#3756009 , @tejohnson wrote: > @davidxl @xur for review and thoughts. > > I'm a little wary of requiring that both pieces of metadata be carried > together, as that seems very brittle to maintain in the compiler. Wh

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. In D131306#3756087 , @tejohnson wrote: > Well I was thinking the extra field would be optional as well and could be > removed. But understood that this requires more changes (although maybe not > if it is optional, and after y

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth added a comment. I think that may be a better approach. Let me investigate a bit and see what the exact tradeoffs are. If anyone else has a suggestion or thought on another way to handle this, I'm all ears. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D133732: [clang-doc] Support default args for functions.

paulkirth added a comment. Thanks for the patch. This is mostly LGTM modulo a few nits. My one question is regarding documentation. Do you think this needs to be described in the clang-tools-extra/doc/clang-doc.rst? And are there any changes in workflow or tool usage that we should document? I

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 418025. paulkirth added a comment. Add missing GenerateArgs call to propagate flags to the backend outside of cc1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 Fi

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1554 + Twine(*Opts.DiagnosticsMisExpectTolerance), SA); + for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeRecover)) I really don't understand wh

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 418280. paulkirth added a comment. Fix check for tolerance diagnostic since we now use GenerateArgs the same way as hotness threshold. Since it has a default value, we need to also ensure that it uses a pattern similar to other options. Repository: rG

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 418328. paulkirth added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 Files: clang/docs/MisExpect.rst clang/docs/ReleaseNotes.rst clang/in

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added a comment. @tejohnson thanks for pointing me to the document. I knew it had something to do w/ CC1 but missed that this was well documented. Is there anything else that needs to be done, or do you think this is good to land again? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rG2add3fbd976d: [misexpect] Re-implement MisExpect Diagnostics (authored by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://revi

[PATCH] D122623: [docs][misexpect] Fix malformed table in docs

paulkirth updated this revision to Diff 418735. paulkirth added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. update table in clang docs too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122623/new/ https://revie

[PATCH] D122623: [docs][misexpect][NFC] Fix malformed table in docs

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa427e18896de: [docs][misexpect][NFC] Fix malformed table in docs (authored by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D122636: [llvm][clang][NFC] Add missing references to MisExpect in TOC

paulkirth created this revision. Herald added a subscriber: arphaman. Herald added a project: All. paulkirth requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added a comment. reverted. I will fix this tomorrow. Sorry for the trouble Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 ___ cfe-commits mailing lis

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 418924. paulkirth added a comment. Herald added a subscriber: arphaman. Fix missing reference in TOC & fix formatting of tables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.o

[PATCH] D122636: [llvm][clang][NFC] Add missing references to MisExpect in TOC

paulkirth abandoned this revision. paulkirth added a comment. Since I had to revert, these changes will be incorporated into the main patch when it lands again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122636/new/ https://reviews.llvm.org/D122

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 419181. paulkirth added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 Files: clang/docs/MisExpect.rst clang/docs/ReleaseNotes.rst clang/do

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

paulkirth updated this revision to Diff 419222. paulkirth added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119996/new/ https://reviews.llvm.org/D119996 Files: clang/test/Frontend/stack-usage-safestack.c llvm/include/llvm/C

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

paulkirth updated this revision to Diff 419493. paulkirth added a comment. Herald added a subscriber: pengfei. Update tests. - Dont rely on size of `int` based on platform - Add checking to backend tests for warn-stack-size to ensure the behavior is consistant when safestack is enabled Reposit

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG46774df30715: [misexpect] Re-implement MisExpect Diagnostics (authored by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added a comment. @jgorbe Sorry for the trouble. Thank you for the backtrace and the revert. Indeed, it seems like I've missed an assumption w.r.t. over/underflow, and will have to sort that out before re-landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D130807: [InstrProf] Add the omitprofile attribute

paulkirth added a comment. Do you expect the difference between `noprofile` and `omitprofile` to be confusing to users? I can certainly see how users could be confused by what the difference is between `noprofile` and `omitprofile` ... Since what you want to communicate is "never profile"(which

[PATCH] D130807: [InstrProf] Add the omitprofile attribute

paulkirth added a comment. > I agree that `omitprofile` probably doesn't communicate the correct meaning > very well. I like `nodirectprofile` because it hopefully implies that > indirect profiles are allowed. I'm wondering if anyone else has suggestions. > > Is there any precedent for renamin

[PATCH] D131306: [llvm][misexpect] Track provenance of branch weights

paulkirth created this revision. paulkirth added a reviewer: tejohnson. Herald added subscribers: ormris, okura, kuter, hiraditya. Herald added a project: All. paulkirth requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a reviewer: sstefan1. Herald added projects:

[PATCH] D131298: [clang-doc] Read docstrings for record members

paulkirth requested changes to this revision. paulkirth added a comment. This revision now requires changes to proceed. I agree w/ @phosek on unit testing. Additionally, while we don't have many of them in clang-doc, an end-to-end test would also be welcome. In this case, it will also probably

[PATCH] D131298: [clang-doc] Read docstrings for record members

paulkirth added inline comments. Comment at: clang-tools-extra/clang-doc/Serialize.cpp:439 +static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) { + ASTContext& Context = D->getASTContext(); + RawComment *Comment = Context.getRawCommentForDeclNoCache(D); --

[PATCH] D131298: [clang-doc] Read docstrings for record members

paulkirth added a comment. LGTM, modulo the small fixes I've left inline Comment at: clang-tools-extra/clang-doc/Serialize.cpp:439 +static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) { + assert(D); + ASTContext& Context = D->getASTContext(); ---

[PATCH] D131298: [clang-doc] Read docstrings for record members

paulkirth added a comment. Thanks. Can we get the small changes to the assert + the TODO in Serialize.cpp? I'll be happy to land this for you after that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131298/new/ https://reviews.llvm.org/D131298

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 414230. paulkirth added a comment. Consolodate common code, clarify documentation, and address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 Files: cla

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth marked 6 inline comments as done. paulkirth added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:1203 + if (CodeGenOpts.MisExpect) { +Ctx.setMisExpectWarningRequested(true); tejohnson wrote: > Out of curiosity, since I am less f

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added a comment. MisExpect was originally intended to be quite strict, so that developers would audit their code and re-evaluate the correctness of their annotations,or if they were needed at all. I think I'm still of a mind that getting flagged by MisExpect indicates that a differen

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 415990. paulkirth added a comment. Address comments. Added clang documentation. Fixed various typos. Updated tests per comments for clang and LLVM. Removed dead class definition. Removed clamping from tolerance parser. Repository: rG LLVM Github Mon

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth marked 10 inline comments as done. paulkirth added inline comments. Comment at: clang/test/Profile/misexpect-branch.c:25 + int x = 0; + if (likely(rando % (outer_loop * inner_loop) == 0)) { // exact-warning-re {{Potential performance regression from use of __builtin_

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth updated this revision to Diff 416356. paulkirth marked 4 inline comments as done. paulkirth added a comment. Fixup remaining outstanding issues - fix typos - add misexpect to clang release notes - restore new pm style opt invocations in llvm tests - move single header function into impl

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth marked 5 inline comments as done. paulkirth added inline comments. Comment at: llvm/docs/MisExpect.rst:54 + +.. option:: -pgo-warn-misexpect + tejohnson wrote: > paulkirth wrote: > > tejohnson wrote: > > > Looking at the code, the -pgo-warn-misexpect se

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe7749d4713a5: [misexpect] Re-implement MisExpect Diagnostics (authored by paulkirth). Changed prior to commit: https://reviews.llvm.org/D115907?vs

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added a comment. These pass for me locally, so I'm reverting for now and will dig into this tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115907/new/ https://reviews.llvm.org/D115907 ___

[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

paulkirth added a comment. Hi, this is also breaking Fuchsia's clang CI builders (https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8816531831869640417/overview). If this will be hard to address, would you mind reverting until a patch is ready? Repository: rG

[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

paulkirth added a comment. Ah, thanks. I've had to try to re-land this so many times, I wanted to be sure the pre-submit was looking OK after rebasing. or at least be sure it didn't look like it was failing from any of my changes. w.r.t. `clamp`, keeping it compatible was my intent. Repositor

[PATCH] D123840: [Clang][Sema] Fix invalid redefinition error in if/switch/for statement

paulkirth added a comment. Hi I think this patch is the root cause of https://github.com/llvm/llvm-project/issues/54968 The issue first appears in our builders here: https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-release/b8816803488685972465/overview based on

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. paulkirth marked an inline comment as done. Closed by commit rG61e36e87df1a: [safestack] Support safestack in stack size diagnostics (authored by paulkirth). Repositor

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

paulkirth added a comment. In D119996#3465735 , @vvereschaka wrote: > Hi @paulkirth, > > using of specific triple within `stack-usage-safestack.c` test causes a > failure for the compilers, which don't support these triples (arm/aarch64 in > my case).

[PATCH] D124203: [clang][safestack] Remove triple from stack usage test

paulkirth created this revision. Herald added a project: All. paulkirth requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Supplying the target triple caused breakeages for compilers that don't support the supplied triple. Repository: rG L

[PATCH] D119996: [safestack] Support safestack in stack size diagnostics

paulkirth added a comment. @vvereschaka I have a fix out in D124203 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119996/new/ https://reviews.llvm.org/D119996 ___ cfe-c

[PATCH] D124203: [clang][safestack] Remove triple from stack usage test

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG414f84ba29d9: [clang][safestack] Remove triple from stack usage test (authored by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D124203: [clang][safestack] Remove triple from stack usage test

paulkirth added a comment. unfortunately it breaks many other tests, so we probably need to revert this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124203/new/ https://reviews.llvm.org/D124203 ___ cfe-

[PATCH] D124210: Revert "[clang][safestack] Remove triple from stack usage test"

paulkirth created this revision. Herald added a project: All. paulkirth requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This reverts commit 414f84ba29d96c8cbbe198cfc022146e4582cbef

[PATCH] D124210: Revert "[clang][safestack] Remove triple from stack usage test"

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb8786413d800: Revert "[clang][safestack] Remove triple from stack

[PATCH] D118428: [clang-cl] Support the /JMC flag

paulkirth added a comment. Hi, We have two failing test cases on Fuchsia's clang canary builder on Windows x64. LLVM :: Instrumentation/JustMyCode/jmc-instrument-x86.ll LLVM :: Instrumentation/JustMyCode/jmc-instrument.ll First seen here: https://luci-milo.appspot.com/ui/p/fuchsia/builders/to

[PATCH] D119574: [clang] Expose -fprofile-use in clang-cl

paulkirth added a comment. Hi, I think we hit a test failure due to this patch in Fuchsia's Clang canary for Windows. We've had a lot of breakages on Windows today, so this was hidden until 0574b5fc landed. The failing test

[PATCH] D119574: [clang] Expose -fprofile-use in clang-cl

paulkirth added a comment. Sorry, that bot is green now as of 437d4e01fe4c057509dff30efd560049ad07bc99 , so it looks like it was just bad timing that led me to suspect your commit, since it touched the test file. I'm not sure

[PATCH] D130226: [clang-doc] Default to Standalone executor and improve documentation

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM w/ the caveat that I think we should update the release notes, since we're changing the default behavior of a command line tool. I've also left a few small suggestions the wording.

[PATCH] D130279: [clang-doc] Add check for pointer validity

paulkirth created this revision. paulkirth added reviewers: phosek, abrachet. Herald added a project: All. paulkirth requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. clang-doc would SEGV when running over the Fuchsia code base. T

[PATCH] D130231: [WIP][clang-doc] Improve the Markdown output

paulkirth added a comment. Overall I like the direction of the patch, I know this is WIP, but I've left some small question inline regarding scope and work tracking. Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:23-52 +static std::string genEscaped(StringRef Name) {

[PATCH] D130279: [clang-doc] Add check for pointer validity

This revision was automatically updated to reflect the committed changes. Closed by commit rG30360d88d422: [clang-doc] Add check for pointer validity (authored by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130279/new/ https://reviews.

[PATCH] D120727: [libc++] Overhaul how we select the ABI library

paulkirth added a comment. Hi, we're seeing some failures in Fuchsia's Clang CI. Our runtimes build seems to be unable to find `cxxabi.h`. The failing bot can be found here: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8814278370664903633/overview Reposito

[PATCH] D125570: [CMake] Disable libedit in Fuchsia toolchain

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125570/new/ https://reviews.llvm.org/D125570

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

paulkirth added a comment. Hi, we're seeing a breakage in Fuchsia's clang CI for x64 windows that I think is related to this patch. https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-windows-x64/b8813962058917346337/overview We see a test failure in Clang :: CodeGen/debug-in

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

paulkirth added a comment. Thanks for the quick response. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123534/new/ https://reviews.llvm.org/D123534 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

paulkirth added a comment. We noticed when building Fuchsia using ToT LLVM that CGo has trouble with the new changes to DWARF introduced in this patch. It appears as if CGo is confused because it found a `DW_TAG_variable` that doesn't have a name, which is allowed by the DWARF standard. This

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth added a subscriber: mcgrathr. paulkirth added a comment. @rnk the standard even has an example of this exact behavior, so I think it's hard to say its //wrong// for LLVM to do this, but that may be more gentle. I'm going to defer to more expert opinions here, however, and loop in @mcgr

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth added a comment. I'm going to try to summarize an offline discussion w/ @mcgrathr about this here: There are some other considerations to think about w.r.t. emitting names for non-source language constructs, as would be the case here. In fact, DWARF already handles this case: the "th

[PATCH] D126224: Add DWARF string debug to clang release notes.

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/docs/ReleaseNotes.rst:447 + structures *without* a ``DW_AT_name`` field, which is valid DWARF, but may + lead to assertion failures in some so

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

paulkirth updated this revision to Diff 216274. paulkirth added a comment. Herald added a subscriber: ormris. Update LLVM based implementation Adds LLVM based tests Cleans up existing tests in clang Adds new llvm flag -pgo-warn-misexpect Adds llvm flag when -Wmisexpect enabled in frontend Adds ne

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

paulkirth added a reviewer: nickdesaulniers. paulkirth added a comment. Add Nick Desaulniers as an additional reviewer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 _

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

paulkirth updated this revision to Diff 217282. paulkirth marked 3 inline comments as done. paulkirth added a comment. Address Code Review - Give better names to extracted constants - Remove extraneous call to getValue() - Make integers const - Remove extra braces - Move NOps into branch - Use st

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

paulkirth marked 5 inline comments as done. paulkirth added inline comments. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:108 + auto *L = mdconst::dyn_extract(MisExpectData->getOperand(2)); + auto *U = mdconst::dyn_extract(MisExpectData->getOperand(3)); + -

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

paulkirth updated this revision to Diff 217285. paulkirth marked an inline comment as done. paulkirth added a comment. Revert to use of auto when extracting metadata Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/

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

paulkirth marked 2 inline comments as done. paulkirth added inline comments. Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:123 + const uint64_t CaseTotal = std::accumulate( + Weights.begin(), Weights.end(), (uint64_t)0, std::plus()); + const int NumUnlike

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

paulkirth updated this revision to Diff 217479. paulkirth added a comment. Removes unused constructor, reformat code, and remove braces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include

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

paulkirth marked 6 inline comments as done. paulkirth added inline comments. Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1009 +public: + DiagnosticInfoMisExpect(const Function &Fn, const Twine &Msg, + const DiagnosticLocation &Loc = DiagnosticLocat

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

paulkirth added a comment. In D65300#1647746 , @lebedev.ri wrote: > Please can you clarify hat's the current layout of these patches? > Is this patch required, or is it superseded by D66324 > (and thus should be abandone

  1   2   3   4   >