Page MenuHomePhabricator

hoyFB (Hongtao Yu)
Software Engineer at Facebook

Projects

User does not belong to any projects.

User Details

User Since
Feb 7 2020, 1:31 PM (42 w, 5 d)

Recent Activity

Aug 26 2020

hoyFB added a comment to D86332: [SampleFDO] Enhance profile remapping support for inline instance.

Looks even better now!

Aug 26 2020, 9:04 AM · Restricted Project

Aug 17 2020

hoyFB committed rGde0c7a044b24: [llvm-objdump] Attempt to fix html doc generation issue. (authored by hoyFB).
[llvm-objdump] Attempt to fix html doc generation issue.
Aug 17 2020, 6:06 PM
hoyFB closed D86123: [llvm-objdump] Attempt to fix html doc generation issue..
Aug 17 2020, 6:06 PM · Restricted Project
hoyFB committed rG819b2d9c7901: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff. (authored by hoyFB).
[llvm-objdump] Symbolize binary addresses for low-noisy asm diff.
Aug 17 2020, 4:55 PM
hoyFB closed D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Aug 17 2020, 4:55 PM · Restricted Project

Aug 12 2020

hoyFB accepted D85538: [Clang options] Optimize optionMatches() runtime by removing mallocs.

LGTM, thanks for the fix!

Aug 12 2020, 3:33 PM · Restricted Project
hoyFB added a reviewer for D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools: davidxl.
Aug 12 2020, 2:46 PM · Restricted Project, Restricted Project

Aug 11 2020

hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Aug 11 2020, 10:38 PM · Restricted Project
hoyFB added a comment to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

The last update looks good. Let's wait a bit for binutils' comment.. I also have a question about whether we should name the option --symbolize-operands, or is there a better name. It drops the existing offset/address operand as well as adding a symbol.

Aug 11 2020, 10:37 PM · Restricted Project
hoyFB added a comment to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

I have seen a similar feature in GNU objdump. FWIW CCed you on https://sourceware.org/pipermail/binutils/2020-August/112803.html

Aug 11 2020, 1:50 PM · Restricted Project
hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Aug 11 2020, 1:48 PM · Restricted Project
hoyFB added inline comments to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Aug 11 2020, 1:47 PM · Restricted Project
hoyFB added a comment to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

@MaskRay How does this change look to you? Please let me know your comments. Thanks!

Aug 11 2020, 9:41 AM · Restricted Project

Aug 10 2020

hoyFB added inline comments to D85664: [SampleFDO] Stop letting findCalleeFunctionSamples return unrelated profiles for invoke instructions .
Aug 10 2020, 10:51 AM · Restricted Project
hoyFB accepted D85664: [SampleFDO] Stop letting findCalleeFunctionSamples return unrelated profiles for invoke instructions .
Aug 10 2020, 10:47 AM · Restricted Project

Aug 7 2020

hoyFB accepted D83852: [llvm-profdata] Implement llvm-profdata overlap for sample profiles.
Aug 7 2020, 11:00 AM · Restricted Project
hoyFB added inline comments to D83852: [llvm-profdata] Implement llvm-profdata overlap for sample profiles.
Aug 7 2020, 11:00 AM · Restricted Project

Aug 4 2020

hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Aug 4 2020, 2:46 PM · Restricted Project
hoyFB added inline comments to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Aug 4 2020, 2:43 PM · Restricted Project

Aug 2 2020

hoyFB added a comment to D82822: [OpenMP][FIX] Consistently use OpenMPIRBuilder if requested.

@davezarzycki The bots reported this as well, didn't build MLIR locally, this should have been fixed by 4d83aa4771d84940626d86c883193af390812281

I was seeing the same issue with what @davezarzycki has encountered. Not sure why this is related to MLIR. The failure is cause by this line:

74: // CHECK-DEBUG-NEXT:    [[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* **@12**), !dbg !{{[0-9]*}}

where @12 is supposed to be defined as

@11 = private unnamed_addr constant [90 x i8] c";llvm-project/clang/test/OpenMP/irbuilder_nested_parallel_for.c;parallel_for_1;85;1;;\00", align 1
@12 = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([90 x i8], [90 x i8]* @11, i32 0, i32 0) }, align 8

The exact match of @12 seems a bit fragile to me since global data names could be changed from time to time. Can you please make it bit more robust? Thank

I just created D85099 to eliminate the problem permanently. Feel free to take a look.

Aug 2 2020, 2:09 PM · Restricted Project
hoyFB added a comment to D82822: [OpenMP][FIX] Consistently use OpenMPIRBuilder if requested.

@davezarzycki The bots reported this as well, didn't build MLIR locally, this should have been fixed by 4d83aa4771d84940626d86c883193af390812281

Aug 2 2020, 1:24 AM · Restricted Project

Jul 31 2020

hoyFB committed rGd23c1d6a8ddd: [AutoFDO] Avoid merging inlinee samples multiple times (authored by hoyFB).
[AutoFDO] Avoid merging inlinee samples multiple times
Jul 31 2020, 9:30 AM
hoyFB closed D84997: [AutoFDO] Avoid merging inlinee samples multiple times.
Jul 31 2020, 9:30 AM · Restricted Project
hoyFB updated the summary of D84997: [AutoFDO] Avoid merging inlinee samples multiple times.
Jul 31 2020, 9:06 AM · Restricted Project

Jul 30 2020

hoyFB added a comment to D84997: [AutoFDO] Avoid merging inlinee samples multiple times.
In D84997#2186518, @wmi wrote:

The multiple inline instance copies generated from loop unrolling or jump threading should have their own FunctionSamples objects. Why merging multiple inline instance to the outline profile will trigger assertion? Could you have a testcase for it?

It's a slightly difference case. In the training build, we didn't unroll, so there's a single inline site in the loop, but in the PGO optimizing build, the loop got unrolled with call site duplicated. And the duplicated call sites all point to the same FunctionSamples, in which case we should merge them only once. Agree that we should add a test case.

Jul 30 2020, 11:17 PM · Restricted Project
hoyFB updated the diff for D84997: [AutoFDO] Avoid merging inlinee samples multiple times.

Updating D84997: [AutoFDO] Avoid merging inlinee samples multiple times

Jul 30 2020, 11:15 PM · Restricted Project
hoyFB added a comment to D84997: [AutoFDO] Avoid merging inlinee samples multiple times.
In D84997#2186553, @wmi wrote:

The multiple inline instance copies generated from loop unrolling or jump threading should have their own FunctionSamples objects. Why merging multiple inline instance to the outline profile will trigger assertion? Could you have a testcase for it?

It's a slightly difference case. In the training build, we didn't unroll, so there's a single inline site in the loop, but in the PGO optimizing build, the loop got unrolled with call site duplicated. And the duplicated call sites all point to the same FunctionSamples, in which case we should merge them only once. Agree that we should add a test case.

Thanks. So it is a combined version of normal PGO and sampleFDO, or the PGO here means the instrument hook for sampleFDO?

For the second problem, we got a bug report for the same problem. I create a testcase and a fix in https://reviews.llvm.org/D84994. I think your fix is more concise while mine is a straightforward fix. If you think your fix is better, free to use the testcase in D84994. Otherwise, I can commit that one.

I will commit Hongtao's version in D84994 so the two fixes can be separated.

Jul 30 2020, 11:14 PM · Restricted Project
hoyFB added reviewers for D84997: [AutoFDO] Avoid merging inlinee samples multiple times: wenlei, wmi.
Jul 30 2020, 6:24 PM · Restricted Project
hoyFB requested review of D84997: [AutoFDO] Avoid merging inlinee samples multiple times.
Jul 30 2020, 6:23 PM · Restricted Project
hoyFB added a comment to D84715: [FIX] Avoid creating BFI when emitting remarks for dead functions.

I'm guessing these are not the only passes that will choke with empty/dead functions. If this is because ORE is being used for a special case, then we might want to fix the special case on the user side.

If we want passes to absorb such empty functions, then there needs to be consistency for all passes. I don't think passes need to absorb such cases though, as this is really a "limbo state" of functions when being removed (a normal empty function should still have entry block and ret instruction).

Agree. The proposed checks are only placed along the particular call path that leads to the failure. If we are going cover all the analyses/passes, the overall effort could be substantial.

If the big assumption is that there shouldn't be an empty body function to deal with, we can probably change how ORE is created at the beginning. Instead of creating a heavy-weight version of ORE (with all dependent analyses invoked), using the light-weight version OptimizationRemarkEmitter ORE(F, nullptr) at LTO.cpp:801.

Jul 30 2020, 5:26 PM · Restricted Project

Jul 28 2020

hoyFB committed rG46ebb619bf0f: [FIX] Resolve test failure in polly/test/ScopInfo/memcpy-raw-source.ll (authored by weiwang).
[FIX] Resolve test failure in polly/test/ScopInfo/memcpy-raw-source.ll
Jul 28 2020, 9:16 AM
hoyFB closed D84720: [FIX] Resolve test failure in polly/test/ScopInfo/memcpy-raw-source.ll.
Jul 28 2020, 9:16 AM · Restricted Project

Jul 27 2020

hoyFB added a comment to D80765: [ELF] Handle bitcode comdat groups separately to deduplicate thinlto comdat sections.

Yeah - one alternative here would be for ThinLTO To "home" debug type information, the same way I believe it "homes" inline functions. (original/incoming IR to the thinlink may have multiple definitions of inline functions - one in every module that has a call to the inline function - but something in the think link summary basically says "drop this definition" in all but one of those modules and in that one blessed module it says "always produce a weak definition of this function, even if you don't need it" - something like that should be done with type information, which would remove the redundancy in the object files)

In the case of a full executable link (with the homing above implemented), I'd then suggest not enabling type units, since they just add overhead and wouldn't reduce any duplication - in the case of a static library built with ThinLTO (if that's even a thing), then you'd still potentially want to use type units so they could be deduplicated against other libraries.

Enabling compile-time type deduplication is indeed an ideal solution, as you pointed out finding a prevailing symbol definition and dropping others for duplicate functions from different modules. This would require quite some work in parsing type metadata on the IR and might slow down thinLink.

Fair point - though presumably the first stage compile would do the metadata walk through the IR, provide a list of types in the thin summary file and the thin link would then communicate to the backend compiles specifying which types each backend compile should emit into its respective object file.

Also this may not completely address deduplication of new data introduced during LTO postLink code generation time. The type unit level deduplication could be the last resort to handle the duplicates, like what is done in non-LTO build.

Not sure I follow - what duplication would remain if the thin link homed type descriptions? When linking to non-LTO (normal machine code/object files) libraries? Yeah, fair point (tangential warning: LLVM's type unit hashes are non-DWARF-conforming and not compatible with GCC's type unit hashes... :/)

It /sounds/ like, maybe the original fix in D62884 maybe was going in the wrong direction, and instead ThinLTO should be removing the comdat group from "homed" inline functions? Then there wouldn't be a need for a special case in how comdats are handled.

I am with @dblaikie here. A bit of archaeology finds that rG4de44b7ef87bcef83798eee69fdcbfab9866d52e was probably going in the wrong direction but it was indeed the simplest/least intrusive approach.
D62884 just made some refactorings. Just a thought: for ThinLTO backend, dropping comdat in processGlobalsForThinLTO may achieve some results.

Yeah, maybe something near/similar to https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Utils/FunctionImportUtils.cpp#L275 - where the comdat is imported as available externally, and its comdat group is stripped. Theory is, maybe we should be stripping the comdat on the other side too.

Hmm, maybe not, though? What about non-whole-program-ThinLTO? There could be a comdat copy of this function in a static library that it should be deduplicated against?

Jul 27 2020, 3:42 PM · Restricted Project

Jul 25 2020

hoyFB added a comment to D80765: [ELF] Handle bitcode comdat groups separately to deduplicate thinlto comdat sections.

Yeah - one alternative here would be for ThinLTO To "home" debug type information, the same way I believe it "homes" inline functions. (original/incoming IR to the thinlink may have multiple definitions of inline functions - one in every module that has a call to the inline function - but something in the think link summary basically says "drop this definition" in all but one of those modules and in that one blessed module it says "always produce a weak definition of this function, even if you don't need it" - something like that should be done with type information, which would remove the redundancy in the object files)

In the case of a full executable link (with the homing above implemented), I'd then suggest not enabling type units, since they just add overhead and wouldn't reduce any duplication - in the case of a static library built with ThinLTO (if that's even a thing), then you'd still potentially want to use type units so they could be deduplicated against other libraries.

Jul 25 2020, 12:23 PM · Restricted Project

Jul 24 2020

hoyFB added a comment to D83852: [llvm-profdata] Implement llvm-profdata overlap for sample profiles.

@wmi We'd like to hear about your input on this. Thanks!

Jul 24 2020, 12:08 PM · Restricted Project
hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 24 2020, 9:20 AM · Restricted Project
hoyFB added a comment to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

It looks like you might still have a binary file in the patch, that is no longer needed?

Jul 24 2020, 9:19 AM · Restricted Project

Jul 22 2020

hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 22 2020, 3:15 PM · Restricted Project
hoyFB added inline comments to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Jul 22 2020, 2:22 PM · Restricted Project
hoyFB added inline comments to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Jul 22 2020, 1:34 PM · Restricted Project
hoyFB updated the summary of D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Jul 22 2020, 1:34 PM · Restricted Project
hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 22 2020, 1:33 PM · Restricted Project

Jul 21 2020

hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 21 2020, 10:31 AM · Restricted Project
hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 21 2020, 10:29 AM · Restricted Project
hoyFB added a comment to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Could you post some examples of what the actual difference in output is, please? I don't quite follow from the tests/description what is intended to change as a result of this option.

Also, if you are adding a new option to llvm-objdump, please remember to document it in llvm/docs/CommandGuide/llvm-objdump.rst.

Jul 21 2020, 10:29 AM · Restricted Project
hoyFB updated the summary of D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..
Jul 21 2020, 10:17 AM · Restricted Project

Jul 20 2020

hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 20 2020, 9:16 PM · Restricted Project
hoyFB updated the diff for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff..

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

Jul 20 2020, 10:19 AM · Restricted Project
hoyFB added a reviewer for D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.: wenlei.
Jul 20 2020, 10:17 AM · Restricted Project
Herald added a project to D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.: Restricted Project.
Jul 20 2020, 10:16 AM · Restricted Project

Jul 17 2020

hoyFB added inline comments to D83852: [llvm-profdata] Implement llvm-profdata overlap for sample profiles.
Jul 17 2020, 12:19 AM · Restricted Project

Jul 15 2020

hoyFB committed rGf3731d34faa7: [LoopUnroll] Update branch weight for remainder loop (authored by hoyFB).
[LoopUnroll] Update branch weight for remainder loop
Jul 15 2020, 12:34 PM
hoyFB closed D83187: [LoopUnroll] Update branch weight for remainder loop.
Jul 15 2020, 12:34 PM · Restricted Project
hoyFB added inline comments to D83187: [LoopUnroll] Update branch weight for remainder loop.
Jul 15 2020, 11:35 AM · Restricted Project
hoyFB updated the diff for D83187: [LoopUnroll] Update branch weight for remainder loop.

Updating D83187: [LoopUnroll] Update branch weight for remainder loop

Jul 15 2020, 11:34 AM · Restricted Project
hoyFB added inline comments to D83187: [LoopUnroll] Update branch weight for remainder loop.
Jul 15 2020, 9:59 AM · Restricted Project
hoyFB updated the diff for D83187: [LoopUnroll] Update branch weight for remainder loop.

Updating D83187: [LoopUnroll] Update branch weight for remainder loop

Jul 15 2020, 9:59 AM · Restricted Project

Jul 14 2020

hoyFB updated the diff for D83187: [LoopUnroll] Update branch weight for remainder loop.

Updating D83187: [LoopUnroll] Update branch weight for remainder loop

Jul 14 2020, 11:11 PM · Restricted Project
hoyFB added a comment to D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO.

I've done testing with the following global parameters.

  • The base for the branch is llvmorg-10-init-8655-g94a4a2c97f8
  • Used llvm, clang, lld, and llvm-ar from this branch.
  • The sqlite kvtest program was the test payload.

This test compared an unmodified compiler from the base of the branch with a modified compiler with this patch applied and the loop optimisation passes mentioned above moved to the backend. The results were as follows. All numbers in seconds.

RunModified LTOModified SPGO+LTOUnmodified SPGO+LTO
142.0041.7342.08
242.3039.4942.45
341.2142.4642.49
AVG:41.8441.2342.34

TL;DR the average run using a compiler built with the modified SPGO pipeline is about a second faster. Definitely a positive initial result.

This probably needs to be taken over by someone who cares about full LTO performance (@wristow or @ormris ?). This patch was some cleanup of the full LTO sample PGO pipeline, but has a number of issues I enumerate in the summary.

Given the performance improvements here, I'd like to develop this patch further.

Jul 14 2020, 2:17 PM · Restricted Project, Restricted Project

Jul 5 2020

hoyFB added a comment to D83187: [LoopUnroll] Update branch weight for remainder loop.

@fhahn I'm not sure who is an appropriate code reviewer. Can you please help find one? Thanks!

Jul 5 2020, 10:43 PM · Restricted Project
Herald added a project to D83187: [LoopUnroll] Update branch weight for remainder loop: Restricted Project.
Jul 5 2020, 10:40 PM · Restricted Project
hoyFB added reviewers for D83187: [LoopUnroll] Update branch weight for remainder loop: fhahn, wenlei.
Jul 5 2020, 10:40 PM · Restricted Project
hoyFB added a comment to D83176: [OpenMPIRBuilder][Fix] Move llvm::omp::types to OpenMPIRBuilder..

Thanks for the quick turnaround! I confirm the change fixes our build break.

Jul 5 2020, 5:35 PM · Restricted Project, Restricted Project

Jul 2 2020

hoyFB added inline comments to D82193: [OpenMPOpt] ICV macro definitions.
Jul 2 2020, 5:50 PM · Restricted Project
hoyFB added inline comments to D82193: [OpenMPOpt] ICV macro definitions.
Jul 2 2020, 3:08 PM · Restricted Project
hoyFB added inline comments to D82193: [OpenMPOpt] ICV macro definitions.
Jul 2 2020, 3:08 PM · Restricted Project
hoyFB added inline comments to D82193: [OpenMPOpt] ICV macro definitions.
Jul 2 2020, 2:03 PM · Restricted Project

Jul 1 2020

hoyFB added inline comments to D82193: [OpenMPOpt] ICV macro definitions.
Jul 1 2020, 4:45 PM · Restricted Project

Jun 24 2020

hoyFB accepted D82500: [llvm-profdata] --hot-func-list: fix some style issues in D81800.

Thanks for fixing these issues!

Jun 24 2020, 2:40 PM · Restricted Project
hoyFB accepted D82355: Add --hot-func-list to llvm-profdata show for sample profiles.
Jun 24 2020, 1:01 PM · Restricted Project
hoyFB added inline comments to D82355: Add --hot-func-list to llvm-profdata show for sample profiles.
Jun 24 2020, 12:28 PM · Restricted Project

Jun 23 2020

hoyFB added inline comments to D82355: Add --hot-func-list to llvm-profdata show for sample profiles.
Jun 23 2020, 11:57 PM · Restricted Project

Jun 17 2020

hoyFB added a comment to D80765: [ELF] Handle bitcode comdat groups separately to deduplicate thinlto comdat sections.

Apologies for my slowness getting to this patch.

symtab->ltoOutputComdatGroups does work:

% readelf -g a3.o.lto.o a3.o1.lto.o 

File: a3.o.lto.o

COMDAT group section [    4] `.group' [4068369915778327548] contains 2 sections:
   [Index]    Name
   [    5]   .debug_types
   [    6]   .rela.debug_types

File: a3.o1.lto.o

COMDAT group section [    4] `.group' [4068369915778327548] contains 2 sections:
   [Index]    Name
   [    5]   .debug_types
   [    6]   .rela.debug_types

The .debug_types from a3.o1.lto.o can be discarded by the new COMDAT logic. However, I am concerned that making it general as this patch does can miss some codegen bugs. See @pcc's argument in
in https://reviews.llvm.org/D56015#1339411 . Since .debug_types (notably, a non-SHF_ALLOC section) is the only COMDAT rule this patch will discard, how about special casing .debug_types (i.e. if isLTOOutput && the group is related to .debug_types)?

Jun 17 2020, 11:58 PM · Restricted Project
hoyFB added a comment to D81981: [PGO] Supplement PGO profile with Sample profile.
In D81981#2099825, @wmi wrote:

The index format needs to be backward compatible, so there needs to be some version specific handling there (can be removed later).

I don't understand this part. Could you elaborate it -- why index format is different from raw format in backward compatiblity, and what is the version specific handling?

For function entry bb which have multiple successors, the existing algorithm in FuncPGOInstrumentation<Edge, BBInfo>::getInstrBB will insert the counter in all its successors. My current implementation simply adds a counter in entry block so in that case, it introduces redundent counter.

I can improve it by selecting a successor to not insert counter for it since it can be inferred from the counters surrounding it. With that implemented, I expect the profile size will be unchanged.

The current PGO instrumentation is based on MST.

Yes, it is based on two parts, selecting MST is one part and selecting src/dst node of each non-MST edge to instrument is another part.

Changing the instrumentation may require changes in how the counts of non-MST-edges are calculated (in PGOUseFunc::setInstrumentedCounts). So maybe adjust the MST to remove the sibling edge ?

The change to add entry BB as an instrumented BB is in function getInstrumentBBs which is shared by profile-gen and profile-use, so it will be consistent between profile-gen and profile-use. About adjusting MST to remove sibling edge, I feel it is inconsistent with current goal of MST selection. The goal of selecting MST is to avoid instrumenting the most frequent edges so we can minimize the cost. Removing a successor edge of the entry block is a different goal. Mixing these two goals will make things complicated. I feel it is simpler to add the change in the second part -- selecting between src and dst which node to instrument.

Jun 17 2020, 11:11 PM · Restricted Project
hoyFB added a comment to D81981: [PGO] Supplement PGO profile with Sample profile.

It's an interesting idea to improve the PGO profile quality with sample profiles. Thanks for working on this!

Jun 17 2020, 5:17 PM · Restricted Project

Jun 13 2020

hoyFB added a reviewer for D81800: Add --hot-func-list to llvm-profdata show for sample profiles: wmi.
Jun 13 2020, 4:33 PM · Restricted Project

Jun 10 2020

hoyFB committed rG2638aafe1203: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules. (authored by hoyFB).
[LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules.
Jun 10 2020, 3:38 PM
hoyFB closed D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..
Jun 10 2020, 3:37 PM · Restricted Project
hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Hope @tejohnson can confirm this is good.

Jun 10 2020, 11:40 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules.

Jun 10 2020, 11:08 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules.

Jun 10 2020, 10:31 AM · Restricted Project
hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Addressed feedbacks.

Jun 10 2020, 9:20 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules.

Jun 10 2020, 9:19 AM · Restricted Project
hoyFB retitled D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules. from [LLD][ThinLTO] Add a switch--thinlto-single-module to allow compiling partial modules. to [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..
Jun 10 2020, 9:18 AM · Restricted Project
hoyFB updated the summary of D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..
Jun 10 2020, 9:17 AM · Restricted Project
hoyFB retitled D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules. from [LLD][ThinLTO] A switch to allow compilation of only one module. to [LLD][ThinLTO] Add a switch--thinlto-single-module to allow compiling partial modules..
Jun 10 2020, 9:17 AM · Restricted Project

Jun 8 2020

hoyFB added a reviewer for D80765: [ELF] Handle bitcode comdat groups separately to deduplicate thinlto comdat sections: grimar.
Jun 8 2020, 1:55 PM · Restricted Project

Jun 3 2020

hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Addressed feedbacks.

Jun 3 2020, 9:53 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

Jun 3 2020, 9:53 AM · Restricted Project

Jun 1 2020

hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Rebased.

Jun 1 2020, 11:52 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

Jun 1 2020, 11:52 AM · Restricted Project

May 28 2020

hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

May 28 2020, 5:38 PM · Restricted Project
hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Can you beef up the testing? I.e. ensure that the save temps output files for just the specified module get created. Also please add a test to make sure this interacts well with distributed thinlto build options, i.e. -thinlto-index-only (e.g. that it creates the expected output files just for the specified module).

Not sure if you saw this comment from my earlier reply - please add additional testing as suggested above.

It's added. E.g, the not ls single1.o2 line checks only two intermediate objects are generated, one is the dummy ld-temp.o and the other has the symbol specified in the command line. The IDX checks are for thinlto index testing. Do you want me to add more? Thanks.

Oh I see now that a -thinlto-index-only invocation was added. What you should be testing for is the output file specific to the specified object. E.g. typically there is a *.thinlto.bc file emitted for each input object. We should get only the one corresponding to the specified module in this case.

Also the other test suggestion I had was to make sure that the save-temps output files work as expected (still get the ones corresponding to the specified module, and not for the other modules). There are a number of .bc files emitted for each object with save-temps but it would be fine to check for one of them I think.

May 28 2020, 5:38 PM · Restricted Project
hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Can you beef up the testing? I.e. ensure that the save temps output files for just the specified module get created. Also please add a test to make sure this interacts well with distributed thinlto build options, i.e. -thinlto-index-only (e.g. that it creates the expected output files just for the specified module).

Not sure if you saw this comment from my earlier reply - please add additional testing as suggested above.

May 28 2020, 10:23 AM · Restricted Project
hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Addressed feedbacks.

May 28 2020, 9:50 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

May 28 2020, 9:50 AM · Restricted Project

May 27 2020

hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

May 27 2020, 12:29 PM · Restricted Project
hoyFB added inline comments to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..
May 27 2020, 12:29 PM · Restricted Project
hoyFB added a comment to D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Address feedbacks.

May 27 2020, 11:22 AM · Restricted Project
hoyFB updated the diff for D80406: [LLD][ThinLTO] Add --thinlto-single-module to allow compiling partial modules..

Updating D80406: [LLD][ThinLTO] A switch to allow compilation of only one module.

May 27 2020, 11:22 AM · Restricted Project