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

paulkirth updated this revision to Diff 217491. paulkirth marked 2 inline comments as done. paulkirth added a comment. Remove namespace qualifier & insert check for number of arguments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https:

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

paulkirth updated this revision to Diff 217494. paulkirth added a comment. Change check for minimum number of metadata operands. Branch weights must have at least 3 elements: metadata name, and a list of at least 2 branch weights Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

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

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

paulkirth abandoned this revision. paulkirth added a comment. This is being abandoned in favor of D66324 , which reimplements this logic completely in the LLVM backend. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[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/lib/Transforms/Utils/MisExpect.cpp:150 +// Operand 0 is a string tag "branch_weights" +if (MDString *Tag = cast(MD->getOperand(0))) { + unsigned NOps = MD->getNumOperands();

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

paulkirth updated this revision to Diff 217528. paulkirth added a comment. Add comment to clarify choice of operands and reasoning about profiling metadata Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Fi

[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:157-158 +// In these cases we should not emit any diagnostic related to misexpect. +if (NOps < 3) + return; + nickdesauln

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

paulkirth added a comment. @leonardchan can you revise your review against the latest revision? It looks to me as though you might have reviewed one of the first diffs instead of what is the latest code. For example the files in clang-tools have not been part of this revision for some time. Als

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

paulkirth updated this revision to Diff 218016. paulkirth added a comment. Address nits - Add pointers to uses of auto - Remove redundant cast - Use consistent type for integers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://revi

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

paulkirth updated this revision to Diff 218017. paulkirth added a comment. Actually address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td

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

paulkirth marked an inline comment as done. paulkirth added a comment. Thanks for the review. I'm working on splitting up the patch now. I should be able to address most of your comments, but I left some inline comments for clarification/discussion. Comment at: clang/lib/Cod

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

paulkirth updated this revision to Diff 218164. paulkirth added a comment. Address Review items - minimize debug info in llvm tests - sanitize paths in debug info - use more complete checks in test for LowerExpectIntrisic - remove references to __builtin_expect from backend code - update test

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

paulkirth added a comment. In D66324#1652364 , @lebedev.ri wrote: > Trying to start reviewing this. > The llvm side of the patch is self contained; clang patch should be split > into a dependent review. Looking at this, is it necessary to split up the

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

paulkirth updated this revision to Diff 218168. paulkirth added a comment. Remove extra include from CodeGenFunction.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/Basic/Di

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

paulkirth updated this revision to Diff 218171. paulkirth added a comment. Fix comment for misexpect option 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

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

paulkirth added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3449 + if (Diags.isIgnored(diag::warn_profile_data_misexpect, SourceLocation())) +Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect"); lebedev.ri wrote: > Err, th

[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/test/Transforms/LowerExpectIntrinsic/basic.ll:141 %tobool = icmp ne i64 %expval, 0 -; CHECK: !prof !1 +; CHECK: !prof !2 ; CHECK-NOT: @llvm.expect lebedev.ri wrote:

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

paulkirth updated this revision to Diff 218197. paulkirth added a comment. Update LowerExpectIntrinsic/basic.ll to generate misexpect metadata for a switch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Fi

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

paulkirth updated this revision to Diff 218198. paulkirth added a comment. Add inline checks for misexpect tags to LowerExpectIntrinstic test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/Basic/DiagnosticFrontendKi

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

paulkirth updated this revision to Diff 218200. paulkirth added a comment. Add clang and LLVM tests for __builtin_unpredictable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/B

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

paulkirth created this revision. paulkirth added reviewers: phosek, leonardchan, nickdesaulniers. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mgorny. Herald added projects: clang, LLVM. This is a continuation of the work in D66324 that creates

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

paulkirth updated this revision to Diff 219171. paulkirth added a comment. Address code review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67253/new/ https://reviews.llvm.org/D67253 Files: clang-tools-extra/CMakeLists.txt clang-tools-extra/c

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

paulkirth updated this revision to Diff 219173. paulkirth added a comment. Remove commented out code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67253/new/ https://reviews.llvm.org/D67253 Files: clang-tools-extra/CMakeLists.txt clang-tools-extra/clang-misexpect/CMakeLists.txt c

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

paulkirth updated this revision to Diff 219219. paulkirth added a comment. Improve diagnostic output with profile counts CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66324/new/ https://reviews.llvm.org/D66324 Files: clang/include/clang/Basic/DiagnosticFrontendKinds.td clang/includ

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

paulkirth added inline comments. Comment at: clang/test/Profile/misexpect-branch-cold.c:4 +// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata +// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -ve

[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: clang/test/Profile/misexpect-branch-cold.c:4 +// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata +// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth updated this revision to Diff 468770. paulkirth added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: abrachet, sstefan1, pengfei, ormris. Refactor printing into a dedicated pass - add some basic lit tests - change the printing format to be more machine readable

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth planned changes to this revision. paulkirth added a comment. I plan to add/update the tests, and give the new pass a few cleanups. The test code can probably be much smaller & have its debug info reduced significantly. There is also a lack of documentation. Repository: rG LLVM Gith

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth added a subscriber: rnk. paulkirth added a comment. @rnk This is the prototype I mentioned the other night at the meetup. I'm pretty sure something like the `LiveDebugValues` pass has a mapping of all the debug information to locations that I'd ever need, but I'm not sure how to poke

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth updated this revision to Diff 469446. paulkirth added a comment. Code Cleanups and Test Cases - Replace SmallVector with SmallPtrSet - Add some comments/clarification - Add a larger, more flexible test case There's still room for improvement for both testing and documentation. Reposi

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth updated this revision to Diff 469838. paulkirth added a comment. Misc cleanups and additional tests - replace stringstream formatting w/ formatv - misc cleanups to includes - fix some typos - add tests for arm and x86 - add more interesting test cases - try to reduce test size w/ llvm-r

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth updated this revision to Diff 469840. paulkirth added a comment. git clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: llvm/include/llvm/CodeGen/Passes.h llvm/include/llvm/

[PATCH] D136638: [clang-doc] Fix typedef/using output.

paulkirth added a comment. Can you add a test that checks the `IsUsing == false` case? Otherwise LGTM modulo one small nit. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:396-397 +template <> llvm::Expected getCommentInfo(TypedefInfo *I) { + I->Description.emplace_

[PATCH] D136638: [clang-doc] Fix typedef/using output.

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:396-397 +template <> llvm::Expected getCommentInfo(TypedefInfo *I) { + I->Description.emplace_back(); +

[PATCH] D135488: [codegen] Display stack layouts in console

paulkirth updated this revision to Diff 470983. paulkirth retitled this revision from "[codegen][WIP] Display stack layouts in console" to "[codegen] Display stack layouts in console". paulkirth edited the summary of this revision. paulkirth added a comment. Rebase and update description Reposi

[PATCH] D135488: [codegen] Display stack layouts in console

paulkirth updated this revision to Diff 470984. paulkirth added a comment. Fix pass name in a comment block Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: llvm/include/llvm/CodeGen/Passes.h ll

[PATCH] D135488: [codegen] Display stack layouts in console

paulkirth updated this revision to Diff 470989. paulkirth added a comment. Fix up use of 'auto' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: llvm/include/llvm/CodeGen/Passes.h llvm/include/l

[PATCH] D135488: [codegen] Display stack layouts in console

paulkirth added a comment. Thanks for the feedback. Those are good catches. I'll send out a patch cleaning those up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 __

[PATCH] D135488: [codegen] Display stack layouts in console

paulkirth updated this revision to Diff 471235. paulkirth added a comment. Address comments - fix brancing style - update parameters to use const - use llvm::sort in place of std::sort - remove path strings from debug info in test files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D136894: Add clang-doc readme

paulkirth added a comment. I'm a bit tied up until next week, and looped in some other reviewers. I can take a look on Monday if needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136894/new/ https://reviews.llvm.org/D136894 __

[PATCH] D136894: Add clang-doc readme

paulkirth requested changes to this revision. paulkirth added a comment. This revision now requires changes to proceed. First, I don't think this is a good fit for a `README.txt` based on the content of other clang-tool `README.txt` files. Most of them only outline project scope or point to form

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG99baa10f8f01: [clang-doc] Read docstrings for record members (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CH

[PATCH] D131739: Fix some clang-doc issues.

paulkirth added a comment. would you mind splitting this patch into separate patches? I'd like to keep the fix for the assertion in SmallString separate. the rest seems to make sense together. otherwise LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D131793: Fix assert on startup in clang-doc

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. This is LGTM, but can you add a tag to the summary title/commit message? ie. `[clang-doc] Fix assert on startup in clang-doc` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D131739: Fix some clang-doc issues.

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. Can you update the summary to reflect what's in this change now + add a tag for clang-doc? `[clang-doc] Always emit the TagType for RecordInfo` I can land this and D131793

[PATCH] D131793: [clang-doc] Fix assert on startup

This revision was automatically updated to reflect the committed changes. Closed by commit rG75c7e79464a3: [clang-doc] Fix assert on startup (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131793/new/ http

[PATCH] D131739: [clang-doc] Always emit the TagType for RecordInfo

This revision was automatically updated to reflect the committed changes. Closed by commit rG68266828b170: [clang-doc] Always emit the TagType for RecordInfo (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D131935: [clang][llvm][NFC] Change misexpect's tolerance option to be 32-bit

paulkirth created this revision. paulkirth added reviewers: jloser, tejohnson, davidxl. Herald added a subscriber: hiraditya. Herald added a project: All. paulkirth requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. In D13186

[PATCH] D131935: [clang][llvm][NFC] Change misexpect's tolerance option to be 32-bit

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG656c5d652ce1: [clang][llvm][NFC] Change misexpect's tolerance option to be 32-bit (authored by paulkirth). Repository: rG LLVM Github Monorepo CH

[PATCH] D131137: [clang][misexpect] Add support to clang for profitable annotation diagnostics

paulkirth created this revision. Herald added a subscriber: ormris. Herald added a project: All. paulkirth updated this revision to Diff 450194. paulkirth added a comment. paulkirth added reviewers: tejohnson, nickdesaulniers, phosek. paulkirth published this revision for review. Herald added a pro

[PATCH] D131618: [WIP][Do NOT review] LLD related changes for -ffat-lto-objects support

paulkirth added inline comments. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:5080 + +void llvm::embedBitcodeInFatObject(llvm::Module &M, llvm::MemoryBufferRef Buf) { + // Save llvm.compiler.used and remove it. Since this has been merged into this comm

[PATCH] D131618: [WIP][Do NOT review] LLD related changes for -ffat-lto-objects support

paulkirth added inline comments. Comment at: lld/ELF/InputFiles.cpp:1721 +Expected fatLTOData = IRObjectFile::findBitcodeInMemBuffer(mb); +if (!errorToBool(fatLTOData.takeError())) + return make(*fatLTOData, archiveName, offsetInArchive, /*lazy=*/false); --

[PATCH] D131618: [WIP][Do NOT review] LLD related changes for -ffat-lto-objects support

paulkirth added a comment. I think the test strategy here needs to be: - Format checking -. llvm generates the correct object file sections when configured for fatlto 1. clang generates fatlto objects correctly (i.e. correctly sets up the codegen pipeline in llvm) Comment a

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

paulkirth added a comment. Hi, thanks for taking a look at this. Before we start an in-depth review, can you describe the deficiencies w/ the existing diagnostics, and why they don't meet your needs? Primarily, I'm a little skeptical if taking the same approach as MisExpect is the correct appr

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

paulkirth added a comment. To be clear, I'm not morally opposed to your patch, I just wanted to understand the context more completely and why this is the best approach. And like I said, I can't recall encountering a place where `noinline` was done for performance reasons. Code size and correct

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

paulkirth added a comment. In D132186#3751985 , @tejohnson wrote: > I have seen a few cases where noinline was used for performance, in addition > to other cases like avoiding too much stack growth. Well, I stand corrected. I'm curious about what these

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

paulkirth added a comment. In D132186#3752403 , @modimo wrote: > Same. The instances I've seen is an older codebase where compiler > optimizations were not as powerful and/or purposefully written by engineers > that didn't trust the compiler to do the r

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

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. Lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133732/new/ https://reviews.llvm.org/D133732 ___ cfe-commits mailing list cfe-com

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGe191086bfcb7: [clang-doc] Support default args for functions. (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133

[PATCH] D134055: [clang-doc] Export more enum information

paulkirth added a comment. Can you make the first line of the summary the commit title? It's a bit more descriptive. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:53 +llvm::Error decodeRecord(const Record &R, llvm::APSInt &Field, llvm::StringRef Blob) { + auto

[PATCH] D134055: [clang-doc] Export enum type and value information.

paulkirth added a comment. I think maybe you made the title the first line of the summary instead of the other way around. I was looking for this as the title: `[clang-doc] Add support for explicitly typed enums` Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:71 +

[PATCH] D134055: [clang-doc] Add support for explicitly typed enums

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134055/new/ https://reviews.llvm.org/D134055 ___ cfe-commits mailing list cfe-co

[PATCH] D134055: [clang-doc] Add support for explicitly typed enums

paulkirth added a comment. Sorry, it looks like a pre-submit formatting check failed. can you `git clang-format HEAD~` and re-upload? I can land your change after that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134055/new/ https://reviews.llvm.org/D134055 _

[PATCH] D134055: [clang-doc] Add support for explicitly typed enums

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeaa7b324d5a2: [clang-doc] Add support for explicitly typed enums (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo

[PATCH] D134225: [clang-doc] Centralize TypeInfo creation.

paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. Thanks for the code cleanup. The patch is LGTM, with some small nits. Comment at: clang-tools-extra/clang-doc/Serialize.cpp:240 +TypeInfo getTypeInfoForType(const QualT

[PATCH] D134225: [clang-doc] Centralize TypeInfo creation.

paulkirth added a comment. Also, can you take a look at the premerge check failure? it looks like it had trouble applying your patch, so you may need to rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134225/new/ https://reviews.llvm.org/D13

[PATCH] D132186: Clang: Add a new flag Wmisnoinline for printing hot noinline functions

paulkirth added a comment. @iamarchit123 I think the standard advice is to start w/ the llvm-test-suite and then explore other benchmarks as needed. Also, Clang itself is often a very good starting point. As for profiles, it probably won't be representative, but you could collect the profile u

[PATCH] D134225: [clang-doc] Centralize TypeInfo creation.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4df84ac377df: [clang-doc] Centralize TypeInfo creation. (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134225/ne

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

paulkirth added a comment. @brettw Thanks for the clarification. While I'm more familiar w/ these set of patches, I have to say that that I was also unsure/surprised by some of these changes. If you have concrete plans for cleanups letting us know about those plans in the initial review will he

[PATCH] D134371: [clang-doc] Add typedef/using information.

paulkirth added a comment. Can we also add support for some of these changes to the other backends? I'm fine w/ us delaying that a bit, and tackling those in separate patches, but we shouldn't let one backend get wildly ahead of the others. Comment at: clang-tools-extra/clang

[PATCH] D134371: [clang-doc] Add typedef/using information.

paulkirth accepted this revision. paulkirth added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:540 } - template <> void addChild(NamespaceInfo *I, EnumInfo &&R) { brettw wrote: > pau

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

paulkirth accepted this revision. paulkirth added a comment. I'm good w/ this. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134235/new/ https://reviews.llvm.org/D134235 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

This revision was automatically updated to reflect the committed changes. Closed by commit rG0afc60858e11: [clang-doc] Clean up *Info constructors. (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134235/new

[PATCH] D134371: [clang-doc] Add typedef/using information.

This revision was automatically updated to reflect the committed changes. Closed by commit rGeed22583fd78: [clang-doc] Add typedef/using information. (authored by brettw, committed by paulkirth). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134371/n

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

paulkirth updated this revision to Diff 464332. paulkirth added a comment. rebase and change implementation to include provenance information directly in the MD_prof metadata Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://review

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth created this revision. Herald added subscribers: mgrang, hiraditya. Herald added a project: All. paulkirth requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Issue #58168 describes the difficulty diagnosing stack si

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth added subscribers: nickdesaulniers, phosek. paulkirth added a comment. @nickdesaulniers @phosek You may be interested in this super hacky prototype. There's a bunch of things to improve(tests, output, code structure, plumbing...), but when we emit the diagnostic for `-Wframe-larger-tha

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth added a comment. In D135488#3844117 , @nickdesaulniers wrote: > Thanks for the patch, here's the output I observe from the LKML thread > : > https://paste.debian.net/

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth added inline comments. Comment at: clang/test/Frontend/frame-diags.c:1 +// TODO: This is not really a functional test yet, and needs to be rewritten. +// currently its just a copy of backend-diagnostics.c, and all of the run/test probinson wrote: > I

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

paulkirth updated this revision to Diff 466919. paulkirth added a comment. Add SlotID to output Improve some initialization Still need to do significant code cleanup and testing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://re

[PATCH] D134371: [clang-doc] Add typedef/using information.

paulkirth added a comment. The premerge check is failing, because it can't apply the patch. Often this just means you need to rebase, so can you try rebasing your change and reuploading? Also, do you have commit access? you've made enough changes that its appropriate to request, which should m

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

paulkirth added a comment. Hi, we're seeing a build failure in Fuchsia's Clang CI. We're seeing this on all of our builders: arm64 & x64 linux, mac and windows FAILED: CMakeFiles/clang_rt.builtins-aarch64.dir/cpu_model.c.o /b/s/w/ir/x/w/recipe_cleanup/cxx-rbevgw5lbzc/reclient-cxx-wrapper.sh

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

paulkirth added a comment. Hi, thanks for the fix. that unblocked our builder. Unfortunately, we still see some errors in tests. FAIL: Clang :: Driver/aarch64-features.c (7460 of 16622) TEST 'Clang :: Driver/aarch64-features.c' FAILED Script: --

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

paulkirth added a comment. Thanks! I think we're good now. At least one of our builders is green now. https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8794218564332891457/overview I'll let you know if anything pops up, but this looks squared away to me. Repository:

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

paulkirth updated this revision to Diff 484694. paulkirth retitled this revision from "[codegen] Display stack layouts in console" to "[codegen] Add a remarks based Stack Layout Analysis pass". paulkirth edited the summary of this revision. paulkirth added a comment. Herald added subscribers: pcwa

[PATCH] D137838: [Support] Move TargetParsers to new component

paulkirth added a comment. Hi, we're seeing some build failures after this change. Seems like the TargetParser isn't getting added to LLVMExports correctly. Can you take a look and address these? Failing bot https://ci.chromium.org/ui/p/fuchsia/builders/prod/llvm-linux-x64/b8794232527428765009

[PATCH] D137838: [Support] Move TargetParsers to new component

paulkirth added a comment. Sorry for the noise, I missed that that bot is setting a CMake flag that affects LLVM_EXPORTS. Please disregard my earlier message. I just need to update the CMake invocation to fix this, so I don't think there is anything required on your end. Repository: rG LLVM

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

paulkirth updated this revision to Diff 486030. paulkirth added a comment. Herald added subscribers: kosarev, kerbowa, jvesely, nemanjai. Add missing pipline test updates for PowerPC and AMDGPU Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/ne

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

paulkirth updated this revision to Diff 486050. paulkirth added a comment. Avoid problems with path separators on windows, and ignore path prefix in diagnostic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135

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

paulkirth added a comment. @thegameg Maybe? It seems straightforward, but I'll need take a look. If it's easy(which I think it willbe), I'll try to update this patch, if it's more complex, I'll probably do a separate patch to add that feature. Comment at: llvm/lib/CodeGen/Sta

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

paulkirth updated this revision to Diff 486094. paulkirth added a comment. Remove unnecesary null pointer check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-r

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

paulkirth updated this revision to Diff 486095. paulkirth added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-remark.c llvm/include/llvm/Cod

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

paulkirth updated this revision to Diff 486112. paulkirth added a comment. Identify the stack protector in output Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-

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

paulkirth added a comment. @arsenm @thegameg @nickdesaulniers @dblaikie @phosek Can we reach a consensus here on how to output this kind of information? I feel like I've been told to move towards remarks as the output method, but that the current diagnostic that tries to lay out the stack visua

[PATCH] D140694: [clang][dataflow] Only model struct fields that are used in the function being analyzed.

paulkirth added a comment. We're seeing a similar breakage on Linux in Fuchsia's Clang CI: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8792772367021497153/overview Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140694

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

paulkirth added a comment. @tejohnson @xur I kind of dropped the ball on these patches, but what are your thoughts on this approach over the old(more invasive) change to the profdata format I had prototyped before? the patch will obviously need to be rebased, but other than that, do we see a do

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

paulkirth added a comment. I think ideally I'd like to surface something like this: A: F26139353: image.png or B: F26139363: image.png The first image uses stdout, and the second uses remarks, but prints everything from

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

paulkirth updated this revision to Diff 487999. paulkirth added a comment. Rebase. - Switch to multi-line remarks - Update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Fronte

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

paulkirth updated this revision to Diff 488023. paulkirth added a comment. Add test for YAML output Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135488/new/ https://reviews.llvm.org/D135488 Files: clang/test/Frontend/stack-layout-remark.c llv

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

paulkirth added a comment. @thegameg I think I finally understood what you meant re: multi-line remarks. Sorry for the back/forth on that, it just didn't click for me until you commented on the screenshot. BTW, is there a way to nest some of the items? Ideally we'd be able to have a `Slot` in

<    1   2   3   4   >