[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

xur updated this revision to Diff 181587. xur added a comment. Update the patch to sync with https://reviews.llvm.org/D54175 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54176/new/ https://reviews.llvm.org/D54176 Files: include/clang/Basic/CodeGenOptions.h include/clang/Driver/Op

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

xur added inline comments. Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:756 + // For COFF, the comdat group name must be the name of a symbol in the + // group. Use the counter variable name. + Cmdt = M->getOrInsertComdat( So

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. lgtm. BTW, I'm in the process of committing D54175 . After that patch, PGO instrumentation can be called after the main inlining. I don't think it will conflict any

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

xur updated this revision to Diff 188955. xur added a comment. Integrated Teresa's suggestion to change the err() to assert. Added documentation change suggested by David. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54176/new/ https://reviews.llvm.org/D54176 Files: docs/UsersManual

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

xur updated this revision to Diff 189167. xur added a comment. Added usage example to UserManual.rst suggested by David. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54176/new/ https://reviews.llvm.org/D54176 Files: docs/UsersManual.rst include/clang/Basic/CodeGenOptions.h inclu

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

This revision was automatically updated to reflect the committed changes. Closed by commit rC355331: [PGO] Clang part of change for context-sensitive PGO (part1) (authored by xur, committed by ). Changed prior to commit: https://reviews.llvm.org/D54176?vs=189167&id=189184#toc Repository: rC

[PATCH] D61022: [ThinLTO] Pass down opt level to LTO backend and handle -O0 LTO in new PM

xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. LGTM. We need to Initialize the OptLevel no matter what. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61022/new/ https://reviews.llvm.org/D61022

[PATCH] D61138: [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass manager

xur created this revision. xur added reviewers: vsk, davidxl. Herald added a subscriber: jfb. Currently InstrProf lowering is not enabled for Clang PGO instrumentation in the new pass manager. The following command "-fprofile-instr-generate -fexperimental-new-pass-manager ..." is broken. This CL

[PATCH] D61138: [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass manager

This revision was automatically updated to reflect the committed changes. Closed by commit rC359215: [PGO] Enable InstrProf lowering for Clang PGO instrumentation in the new pass… (authored by xur, committed by ). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

xur updated this revision to Diff 186023. xur added a comment. Updated the patch to sync with D54175 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54176/new/ https://reviews.llvm.org/D54176 Files: include/clang/Basic/CodeGenOptions.h include/clang/

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

xur marked 3 inline comments as done. xur added a comment. I'm sorry that I missed this review for this long! In D64029#1581952 , @chandlerc wrote: > Sorry for the delay here. > > It'd be nice to land the LLVM patch first and with its own testing -- we >

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

xur updated this revision to Diff 212237. xur marked an inline comment as done. xur added a comment. Integrated Chandler's review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64029/new/ https://reviews.llvm.org/D64029 Files: clang/lib/CodeGen/BackendUtil.cpp clang/test/P

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

xur marked 3 inline comments as done. xur added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1125 +PB.addPGOInstrPassesForO0(MPM, CodeGenOpts.DebugPassManager, + /* RunProfileGen */ PGOOpt->Action == +

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

This revision was automatically updated to reflect the committed changes. xur marked an inline comment as done. Closed by commit rL367628: [PGO] Add PGO support at -O0 in the experimental new pass manager (authored by xur, committed by ). Changed prior to commit: https://reviews.llvm.org/D64029

[PATCH] D63155: [clang][NewPM] Fix broken profile test

xur added a comment. This patch does not make sense to me. The reason of failing with -fexperimental-new-pass-manager is because we don't do PGO instrumentation at -O0. This is due to the fact that PGO instrumentation/use passes are under PassBuilder::buildPerModuleDefaultPipeline. This patch

[PATCH] D63155: [clang][NewPM] Fix broken profile test

xur added a comment. they are not doing the exacly the same thing for old pass manager and new pass manger: old pass manger is doing instrumentation, while the new pass manager with this change is NOT. the test is not check instrumentation, (it only check the variables that generates by InstroP

[PATCH] D63155: [clang][NewPM] Fix broken profile test

xur added a comment. In D63155#1563266 , @chandlerc wrote: > In D63155#1563240 , @leonardchan > wrote: > > > In D63155#1563229 , @xur wrote: > > > > > This patch does not ma

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

xur created this revision. xur added a reviewer: chandlerc. Herald added a subscriber: hiraditya. Herald added a project: LLVM. Add PGO support at -O0 in the experimental new pass manager to sync the behavior with the legacy pass manager. Also change the test of gcc-flag-compatibility.c for more

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

xur updated this revision to Diff 207392. xur marked an inline comment as done. xur added a comment. Sent the wrong test file in last patch. Update to the correct one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64029/new/ https://reviews.llvm.org/D64029 Files: clang/lib/CodeGen/Ba

[PATCH] D64029: [PGO] Add PGO support at -O0 in the experimental new pass manager

xur added a comment. Ping Comment at: llvm/lib/Passes/PassBuilder.cpp:576 +// dramatically increase code size. +MPM.addPass(GlobalDCEPass()); + } I moved this pass under the condition when Early inline is enabled. I'm under the impression that the int

[PATCH] D84261: [PGO] Supporting code for always instrumenting entry block

This revision was automatically updated to reflect the committed changes. Closed by commit rG50da55a58534: [PGO] Supporting code for always instrumenting entry block (authored by xur). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://review

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

xur added a comment. I agreed with Teresa: adding an optional string is better than using a separated metadata here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131306/new/ https://reviews.llvm.org/D131306 __

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdb18f26567be: [llvm-profdata] Handle internal linkage functions in profile supplementation (authored by xur). Herald added a project: clang. Changed

[PATCH] D132991: [Clang] Give error message for invalid profile path when compiling IR

xur added a comment. I'm fine with this change. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1301 +unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error, +"Could not read profile %0: %1"); +llvm::handleAllE

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

xur created this revision. xur added a reviewer: tejohnson. Herald added subscribers: dexonsmith, steven_wu, hiraditya, mehdi_amini. For IR input files, we currently use LLVM diagnostic handler even the compilation is from clang. As a result, we are not able to use -Rpass to get the transformation

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

xur marked 3 inline comments as done. xur added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:594 + // SourceLoc. + if (Context == nullptr) { +return FullSourceLoc(); tejohnson wrote: > Does this only happen with IR input? Does it always

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

xur updated this revision to Diff 237716. xur added a comment. Integrated Teresa's review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72523/new/ https://reviews.llvm.org/D72523 Files: clang/lib/CodeGen/CodeGenAction.cpp clang/test/CodeGen/Inputs/thinlto_expect1.proftext

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

xur updated this revision to Diff 237810. xur added a comment. Thanks Teresa for the suggestion: using the print() function in the llvm diagnosticPrinter is a lot better than duplicating the code. I changed the patch to use print() function. Also added comment as suggested by Teresa. CHANGES S

[PATCH] D72523: [remark][diagnostics] Using clang diagnostic handler for IR input files

This revision was automatically updated to reflect the committed changes. Closed by commit rG60d39479221d: [remark][diagnostics] Using clang diagnostic handler for IR input files (authored by xur). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D72523?vs=237810

[PATCH] D91673: [PGO] Enable preinline and cleanup when optimize for size

xur accepted this revision. xur added a comment. Sorry to the late response (I somehow missed the reply). (1) Can we add some comments to explain the reason for enabling this for -Oz -- I think the comments from @rnk is useful to some people (at least to me). (2) Also please add a testcase. Fe

[PATCH] D91813: [PGO] verify BFI counts after loading profile data

This revision was automatically updated to reflect the committed changes. Closed by commit rG54e03d03a7a4: [PGO] Verify BFI counts after loading profile data (authored by xur). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D90275: [clang][IR] Add support for leaf attribute

xur added a comment. This breaks llvm/test/Bitcode/attributes.ll. The added test code was obviously wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90275/new/ https://reviews.llvm.org/D90275 ___ cf

[PATCH] D92493: [IR] Add hot to function attributes and use hot/cold attribute in function section prefix/suffix

This revision was automatically updated to reflect the committed changes. Closed by commit rG3733463dbb58: [IR][PGO] Add hot func attribute and use hot/cold attribute in func section (authored by xur). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D91673: [PGO] Enable preinline and cleanup when optimize for size

xur added a comment. This is probably OK for -Os (SizeLevel == 1), but we need to be careful with Oz (SizeLevel == 2). We already know that enabling preinliner in general will reduce size -- as the preinliner is pretty conservative. But there will be cases size will be increased. I would like

[PATCH] D96487: Restore diagnostic handler after CodeGenAction::ExecuteAction

xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. This looks fine to me. Thanks for fixing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96487/new/ https://reviews.llvm.org/D96487 __

[PATCH] D107878: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5fdaaf7fd8f3: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO) profile loader (authored by xur). Herald added a project: clang. Herald added a subscrib

[PATCH] D136397: [Clang] Change AnonStructIds in MangleContext to per-function based

xur created this revision. xur added reviewers: rsmith, davidxl. Herald added a subscriber: wenlei. Herald added a project: All. xur requested review of this revision. Clang is generating different mangled names for the same lambda function in build that are slightly different (like from non-rela

[PATCH] D136397: [Clang] Change AnonStructIds in MangleContext to per-function based

xur marked an inline comment as done. xur added inline comments. Comment at: clang/include/clang/AST/Mangle.h:97 + +// If FunctionDecl is passed in, the anonnousstructID will be per-function +// based. rsmith wrote: > Fixed. Thanks! CHANGES SINCE LAST

[PATCH] D136397: [Clang] Change AnonStructIds in MangleContext to per-function based

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. xur marked an inline comment as done. Closed by commit rG6cee5393371f: [Clang] Change AnonStructIds in MangleContext to per-function based (authored by xur). Herald add

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

xur created this revision. xur added reviewers: davidxl, tmsriram. Herald added a subscriber: wenlei. Herald added a project: All. xur requested review of this revision. Herald added a project: LLVM. This patch has the following changes: (1) Handling of internal linkage functions (static functions

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

xur added inline comments. Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:606 +} else { + StaticFuncMap[NewName] = "---"; +} davidxl wrote: > define a macro for the marker string. Will do. Comment at: llvm/tools/llvm-profda

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

xur updated this revision to Diff 455640. xur added a comment. Integrated David's review commends. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132600/new/ https://reviews.llvm.org/D132600 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/ProfileData/SampleProf.h llvm

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

xur updated this revision to Diff 455682. xur added a comment. Fixed unneeded & CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132600/new/ https://reviews.llvm.org/D132600 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/ProfileData/SampleProf.h llvm/test/tools/llvm-pr

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

xur added inline comments. Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:589 + +if (SampleProfileHasFUnique) { + // If profile also uses funqiue, nothing to do here. tmsriram wrote: > Maybe rewrite this slightly as: > > // If Sample Profile an

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

xur updated this revision to Diff 455816. xur added a comment. Integrated Sriraman's suggestion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132600/new/ https://reviews.llvm.org/D132600 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/ProfileData/SampleProf.h llvm/t

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

xur accepted this revision. xur added a comment. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157750/new/ https://reviews.llvm.org/D157750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D62627: [NFC] Do not run CGProfilePass when not using integrated assembler

xur added a comment. look good to me with the fix of comments. Comment at: llvm/include/llvm/Passes/PassBuilder.h:110 + /// Tuning option to enable/disable call graph profile. Its default value is + /// true. + bool CallGrpahProfile; The default value is spe

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

xur created this revision. xur added reviewers: tejohnson, jeroen.dobbelaere. This patch fixes PR44896. For IR input files, option fdiscard-value-names should be ignored as we need named values in loadModule(). Commit 60d3947922

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

xur added a comment. In D74878#1884769 , @serge-sans-paille wrote: > @lebedev.ri maybe we should at least warn the user if there was a flag at > clang level to force discarding? That's what I've been doing in > https://reviews.llvm.org/D74871?id=245594

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

xur updated this revision to Diff 245891. xur added reviewers: mehdi_amini, serge-sans-paille. xur added a comment. Update the patch based on the reviews from Mehdi and Serge. Reuse Sege's warning code in https://reviews.llvm.org/D74871?id=245594. CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

xur added a comment. Gentle ping. Is the newest patch ok? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

xur updated this revision to Diff 246260. xur added a comment. check if the -fdiscard-value-names explicitly in args (suggested by Jeroen and Serge) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 Files: clang/include/clang/Basic/DiagnosticDr

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

xur updated this revision to Diff 246265. xur added a comment. Uploaded a wrong patch in the last time. This is the correct one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74878/new/ https://reviews.llvm.org/D74878 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

This revision was automatically updated to reflect the committed changes. Closed by commit rG11857d49948b: [remark][diagnostics] [codegen] Fix PR44896 (authored by xur). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D748

[PATCH] D28867: [Profile] Warn about out-of-date profiles only when there are mismatches

xur added a comment. This change gonna hide the missing profile for a function permanently if there is no mismatch. Like, if a user adds a new function without changing existing functions, he will never get a warning if using the old profiles. In LLVM IR level PGO, we also find the missing prof

[PATCH] D28867: [Profile] Add off-by-default -Wprofile-instr-missing warning

xur added a comment. Looks good to me. https://reviews.llvm.org/D28867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99821: [PGO, test] Fix typo in FileCheck var

xur accepted this revision. xur 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/D99821/new/ https://reviews.llvm.org/D99821 ___ cfe-com

[PATCH] D94463: [Driver] Fix assertion failure when -fprofile-generate -fcs-profile-generate are used together

xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. looks good to me. Thanks for working on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94463/new/ https://reviews.llvm.org/D94463 __

[PATCH] D108499: [clang][codegen] Don't assert on CurLinkModule for silenced diagnostic

xur added a comment. This looks fine to me. But I agree you that a more complete fix would be having CurLinkModule set for all callers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108499/new/ https://reviews.llvm.org/D108499 ___

[PATCH] D108603: [clang][codegen] Set CurLinkModule in CodeGenAction::ExecuteAction

xur accepted this revision. xur added a comment. This revision is now accepted and ready to land. Thanks for fix this! I like this fix and the patch looks good to me. -Rong Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108603/new/ https://reviews.