Page MenuHomePhabricator

hoy (Hongtao Yu)
Software Engineer at Facebook

Projects

User does not belong to any projects.

User Details

User Since
Feb 23 2020, 3:46 PM (59 w, 1 d)

Recent Activity

Today

hoy added a comment to D100351: [CSSPGO] Fix a test issue due to portablity of std::hash.

Thanks for the fix. How about using DAG check for things under other "Getting base profile for function" too?

Mon, Apr 12, 6:20 PM · Restricted Project
hoy updated the diff for D100351: [CSSPGO] Fix a test issue due to portablity of std::hash.

Rebasing.

Mon, Apr 12, 5:12 PM · Restricted Project
hoy updated the summary of D100351: [CSSPGO] Fix a test issue due to portablity of std::hash.
Mon, Apr 12, 5:09 PM · Restricted Project
hoy requested review of D100351: [CSSPGO] Fix a test issue due to portablity of std::hash.
Mon, Apr 12, 5:03 PM · Restricted Project
hoy added a comment to D99815: [CSSPGO][Test] XFAIL profile-context-tracker-debug.ll on AIX.
In D99815#2684210, @hoy wrote:

So this is not only failing for AIX, it is actually depends on the libc++ implementation. It may fail on Linux if using non default libc++ .

Any plan of fixing the code to not using std::hash<std::string> ? Thanks. @hoy

Thanks for pointing it out. Yes, we think that may be related to libstdc++ function _Hash_bytes. My plan is to tweak the test with Check-DAG since the promoting order of the children should not cause codegen diffs. We could also sort the children by names instead of hash value for a more stable order across platforms, but that''ll likely incur more overhead. What do you think?

namespace std
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION

  // Hash function implementation for the nontrivial specialization.
  // All of them are based on a primitive that hashes a pointer to a
  // byte array. The actual hash algorithm is not guaranteed to stay
  // the same from release to release -- it may be updated or tuned to
  // improve hash quality or speed.
  size_t
  _Hash_bytes(const void* __ptr, size_t __len, size_t __seed);

Thanks, I am OK with tweaking test with check-DAG as long as you can confirm that this is safe in codegen ( I am not familiar with these code enough to judge).

Mon, Apr 12, 3:05 PM · Restricted Project
hoy added a comment to D99815: [CSSPGO][Test] XFAIL profile-context-tracker-debug.ll on AIX.

So this is not only failing for AIX, it is actually depends on the libc++ implementation. It may fail on Linux if using non default libc++ .

Any plan of fixing the code to not using std::hash<std::string> ? Thanks. @hoy

Mon, Apr 12, 2:31 PM · Restricted Project
hoy added reviewers for D100334: [CSSPGO] Exclude pseudo probes from slot index: wenlei, wmi, davidxl.
Mon, Apr 12, 12:14 PM · Restricted Project
hoy updated the diff for D100334: [CSSPGO] Exclude pseudo probes from slot index.

Updating D100334: [CSSPGO] Exclude pseudo probes from slot index

Mon, Apr 12, 12:13 PM · Restricted Project
hoy requested review of D100334: [CSSPGO] Exclude pseudo probes from slot index.
Mon, Apr 12, 12:04 PM · Restricted Project
hoy added reviewers for D100332: [CSSPGO] Flip SkipPseudoOp to true for MIR APIs.: wenlei, wmi, davidxl.
Mon, Apr 12, 11:26 AM · Restricted Project
hoy requested review of D100332: [CSSPGO] Flip SkipPseudoOp to true for MIR APIs..
Mon, Apr 12, 11:24 AM · Restricted Project
hoy added a comment to D100235: [CSSPGO][llvm-profgen] Fixing an obselete iterator issue..

Another issue being fixed is that all outlined functions shared the same inline frame previously due to the unchanged Index value as the dummy inlineSite identifier.

Why is this an issue? What problem does this cause? Conceptually, if we want to give them a tree structure, top level inliner are indeed children of dummy root.

Yes, top level inliner is still children of dummy root with this change. However, they do not share dummy callsite id any more. Note that without this change, the dummy callsite id for top-level inliner (Index below) isn't updated correctly.

if (Root != Cur) {
   Index = readUnsignedNumber<uint32_t>();
 }

Yeah, index would always be zero. But what problem does this cause? It seems that the call site id for top-level inliner is a dummy field anyways, so value should be ignored. Currently it works just fine, right? Is it also motivated by the upcoming change to report dangling probes with counts (so you need to different call site under dummy root)? If so, it might make sense to include it there as well..

Mon, Apr 12, 10:12 AM · Restricted Project

Yesterday

hoy added a comment to D100235: [CSSPGO][llvm-profgen] Fixing an obselete iterator issue..

Another issue being fixed is that all outlined functions shared the same inline frame previously due to the unchanged Index value as the dummy inlineSite identifier.

Why is this an issue? What problem does this cause? Conceptually, if we want to give them a tree structure, top level inliner are indeed children of dummy root.

Sun, Apr 11, 11:36 PM · Restricted Project

Fri, Apr 9

hoy added reviewers for D100235: [CSSPGO][llvm-profgen] Fixing an obselete iterator issue.: wenlei, wlei, wmi.
Fri, Apr 9, 3:45 PM · Restricted Project
hoy updated the diff for D100235: [CSSPGO][llvm-profgen] Fixing an obselete iterator issue..

Updating D100235: [CSSPGO][llvm-profgen] Fixing an obselete iterator issue.

Fri, Apr 9, 3:45 PM · Restricted Project
hoy requested review of D100235: [CSSPGO][llvm-profgen] Fixing an obselete iterator issue..
Fri, Apr 9, 3:43 PM · Restricted Project
hoy updated the diff for D100075: [CSSPGO] Unblock optimizations with pseudo probe instrumentation part 2..

Rebasing.

Fri, Apr 9, 9:18 AM · Restricted Project

Thu, Apr 8

hoy accepted D100090: [CSSPGO] Fix dangling context strings and improve profile order consistency and error handling.

LGTM, thanks.

Thu, Apr 8, 12:25 PM · Restricted Project
hoy added inline comments to D100090: [CSSPGO] Fix dangling context strings and improve profile order consistency and error handling.
Thu, Apr 8, 11:05 AM · Restricted Project
hoy added a comment to D100090: [CSSPGO] Fix dangling context strings and improve profile order consistency and error handling.

Thanks for fixing the name volatileness issue which was overlooked previously.

Thu, Apr 8, 10:19 AM · Restricted Project

Wed, Apr 7

hoy committed rG2a2720a2dec4: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG… (authored by hoy).
[CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG…
Wed, Apr 7, 10:46 PM
hoy closed D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine..
Wed, Apr 7, 10:46 PM · Restricted Project
hoy updated the diff for D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine..

Removing removeRedundantPseudoProbes calls.

Wed, Apr 7, 10:35 PM · Restricted Project
hoy added inline comments to D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine..
Wed, Apr 7, 10:33 PM · Restricted Project
hoy added inline comments to D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine..
Wed, Apr 7, 5:35 PM · Restricted Project
hoy added reviewers for D100075: [CSSPGO] Unblock optimizations with pseudo probe instrumentation part 2.: wenlei, wmi, davidxl.
Wed, Apr 7, 4:17 PM · Restricted Project
hoy updated the diff for D100075: [CSSPGO] Unblock optimizations with pseudo probe instrumentation part 2..

Updating D100075: [CSSPGO] Unblock optimizations with pseudo probe instrumentation part 2.

Wed, Apr 7, 4:16 PM · Restricted Project
hoy requested review of D100075: [CSSPGO] Unblock optimizations with pseudo probe instrumentation part 2..
Wed, Apr 7, 4:12 PM · Restricted Project
hoy updated the diff for D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine..

Fixing test issue.

Wed, Apr 7, 1:12 PM · Restricted Project

Tue, Apr 6

hoy added reviewers for D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine.: wenlei, wmi, davidxl.
Tue, Apr 6, 6:34 PM · Restricted Project
hoy requested review of D100002: [CSSPGO] Move pseudo probes to the beginning of a block to unblock SelectionDAG combine..
Tue, Apr 6, 6:32 PM · Restricted Project

Mon, Apr 5

hoy accepted D99787: [CSSPGO] Fix incorrect probe distribution factor computation in top-down inliner.

lgtm, thanks!

Mon, Apr 5, 2:28 PM · Restricted Project
hoy added inline comments to D99787: [CSSPGO] Fix incorrect probe distribution factor computation in top-down inliner.
Mon, Apr 5, 11:05 AM · Restricted Project

Fri, Apr 2

hoy added a comment to D99815: [CSSPGO][Test] XFAIL profile-context-tracker-debug.ll on AIX.

Thanks for reporting this issue. I'm wondering if you have tried D99547 which fixed a similar non-determinism issue.

Fri, Apr 2, 2:38 PM · Restricted Project
hoy added inline comments to D95929: [CSSPGO][llvm-profgen] Add brackets for context id to support extended binary format.
Fri, Apr 2, 10:25 AM · Restricted Project
hoy added inline comments to D95929: [CSSPGO][llvm-profgen] Add brackets for context id to support extended binary format.
Fri, Apr 2, 10:08 AM · Restricted Project
hoy added inline comments to D95929: [CSSPGO][llvm-profgen] Add brackets for context id to support extended binary format.
Fri, Apr 2, 10:03 AM · Restricted Project
hoy added inline comments to D95929: [CSSPGO][llvm-profgen] Add brackets for context id to support extended binary format.
Fri, Apr 2, 10:00 AM · Restricted Project

Thu, Apr 1

hoy added inline comments to D99787: [CSSPGO] Fix incorrect probe distribution factor computation in top-down inliner.
Thu, Apr 1, 11:51 PM · Restricted Project
hoy added inline comments to D99787: [CSSPGO] Fix incorrect probe distribution factor computation in top-down inliner.
Thu, Apr 1, 11:28 PM · Restricted Project
hoy added inline comments to D99787: [CSSPGO] Fix incorrect probe distribution factor computation in top-down inliner.
Thu, Apr 1, 11:28 PM · Restricted Project
hoy accepted D99788: [CSSPGO] Skip dangling probe value when computing profile summary.

LGTM, thanks!

Thu, Apr 1, 10:45 PM · Restricted Project

Tue, Mar 30

hoy committed rG3e3fc431dfe4: [CSSPGO] Top-down processing order based on full profile. (authored by hoy).
[CSSPGO] Top-down processing order based on full profile.
Tue, Mar 30, 10:43 AM
hoy closed D99351: [CSSPGO] Top-down processing order based on full profile..
Tue, Mar 30, 10:43 AM · Restricted Project
hoy edited reviewers for D97667: [loop-idiom] Hoist loop memcpys to loop preheader, added: hoy; removed: hoyFB.
Tue, Mar 30, 9:20 AM · Restricted Project
hoy added a comment to D97667: [loop-idiom] Hoist loop memcpys to loop preheader.

Is @zino someone's replacement account?
I'm asking because the accept of this revision is the only contribution of that account.

Tue, Mar 30, 9:20 AM · Restricted Project
hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Updating D99351: [CSSPGO] Top-down processing order based on full profile.

Tue, Mar 30, 9:09 AM · Restricted Project
hoy added inline comments to D99351: [CSSPGO] Top-down processing order based on full profile..
Tue, Mar 30, 9:07 AM · Restricted Project

Mon, Mar 29

hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Addressing comments from Wei and Wenlei.

Mon, Mar 29, 10:39 PM · Restricted Project
hoy added inline comments to D99351: [CSSPGO] Top-down processing order based on full profile..
Mon, Mar 29, 10:38 PM · Restricted Project
hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Updating D99351: [CSSPGO] Top-down processing order based on full profile.

Mon, Mar 29, 6:30 PM · Restricted Project
hoy added a comment to D99351: [CSSPGO] Top-down processing order based on full profile..
In D99351#2657349, @wmi wrote:

The performance test result is neutral, so I think we can enable UseProfiledCallGraph by default.

Mon, Mar 29, 6:30 PM · Restricted Project
hoy added inline comments to D99351: [CSSPGO] Top-down processing order based on full profile..
Mon, Mar 29, 11:39 AM · Restricted Project
hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Moving call graph build into profiledCallGraph.h.

Mon, Mar 29, 11:39 AM · Restricted Project
hoy added a comment to D99351: [CSSPGO] Top-down processing order based on full profile..
In D99351#2654984, @hoy wrote:
In D99351#2654965, @wmi wrote:

Seems there is still build error:

lib/Transforms/IPO/SampleProfile.cpp:1692:23: error: no matching constructor for initialization of 'llvm::sampleprof::ProfiledCallGraph'

ProfiledCallGraph ProfiledCG;

include/llvm/Transforms/IPO/ProfiledCallGraph.h:43:3: note: candidate constructor not viable: requires 2 arguments, but 0 were provided

ProfiledCallGraph(StringMap<FunctionSamples> &ProfileMap,

Oh I see. I didn't sync to Wenlei's latest patch whee the profiled callgraph construction was moved into the constructor. Since there is a subtle difference about whether to limit the call graph build to nodes with samples only, I'm adding back the default constructor for now. We can discuss how to make the code well shared.

Let me commit my change so this can also move forward (sorry for delay). I was thinking about adding another ctor for profiled call graph for AutoFDO case (would require changes in this patch). But we could also move the graph building into a helper function..

Mon, Mar 29, 9:54 AM · Restricted Project

Sun, Mar 28

hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Updating D99351: [CSSPGO] Top-down processing order based on full profile.

Sun, Mar 28, 11:21 PM · Restricted Project
hoy added a comment to D99351: [CSSPGO] Top-down processing order based on full profile..
In D99351#2654965, @wmi wrote:

Seems there is still build error:

lib/Transforms/IPO/SampleProfile.cpp:1692:23: error: no matching constructor for initialization of 'llvm::sampleprof::ProfiledCallGraph'

ProfiledCallGraph ProfiledCG;

include/llvm/Transforms/IPO/ProfiledCallGraph.h:43:3: note: candidate constructor not viable: requires 2 arguments, but 0 were provided

ProfiledCallGraph(StringMap<FunctionSamples> &ProfileMap,
Sun, Mar 28, 11:21 PM · Restricted Project
hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Addressing Wenlei's and Wei's comment.

Sun, Mar 28, 4:27 PM · Restricted Project
hoy added a comment to D99351: [CSSPGO] Top-down processing order based on full profile..
In D99351#2654704, @wmi wrote:
In D99351#2654236, @hoy wrote:
In D99351#2654048, @wmi wrote:

Just find ProfiledCallGraph.h is not included in the patch so the build failed after applying the patch.

Thanks for trying it. You’ll also need to apply Wenlei’s patch D99146.

Then I run into the cyclic including issue:
In this patch llvm/Transforms/IPO/SampleContextTracker.h includes llvm/Transforms/IPO/ProfiledCallGraph.h while in D99146 llvm/Transforms/IPO/ProfiledCallGraph.h includes llvm/Transforms/IPO/SampleContextTracker.h.

Sun, Mar 28, 4:10 PM · Restricted Project

Sat, Mar 27

hoy added a comment to D99351: [CSSPGO] Top-down processing order based on full profile..
In D99351#2654048, @wmi wrote:

Just find ProfiledCallGraph.h is not included in the patch so the build failed after applying the patch.

Thanks for trying it. You’ll also need to apply Wenlei’s patch D99146.

Sat, Mar 27, 11:00 AM · Restricted Project

Fri, Mar 26

hoy committed rG12ac0403b1d9: [CSSPGO][NFC] Fix a debug dump issue. (authored by hoy).
[CSSPGO][NFC] Fix a debug dump issue.
Fri, Mar 26, 4:07 PM
hoy closed D99441: [CSSPGO][NFC] Fix a debug dump issue..
Fri, Mar 26, 4:07 PM · Restricted Project
hoy added a comment to D99441: [CSSPGO][NFC] Fix a debug dump issue..

The printing and the wording were intentional - here we're promoting a context, and it doesn't have to be a context with profile. I wanted to capture the tree transformation, not limited to nodes with profile.

Fri, Mar 26, 3:45 PM · Restricted Project
hoy added a comment to D99351: [CSSPGO] Top-down processing order based on full profile..
In D99351#2653712, @wmi wrote:

The change is an enhancement to https://reviews.llvm.org/D95988 so it is better to mention D95988 in the description/commit log.

Before this patch UseProfileIndirectCallEdges is true for non-CS sampleFDO, now UseProfiledCallGraph is false by default so it change the behavior of non-CS sampleFDO. I will test UseProfiledCallGraph==true for non-CS sampleFDO and if performance is ok, we can enable UseProfiledCallGraph by default. How does it sound?

Fri, Mar 26, 2:00 PM · Restricted Project
hoy updated the summary of D99351: [CSSPGO] Top-down processing order based on full profile..
Fri, Mar 26, 1:57 PM · Restricted Project
hoy added reviewers for D99441: [CSSPGO][NFC] Fix a debug dump issue.: wenlei, wmi.
Fri, Mar 26, 1:19 PM · Restricted Project
hoy requested review of D99441: [CSSPGO][NFC] Fix a debug dump issue..
Fri, Mar 26, 1:18 PM · Restricted Project
hoy updated the diff for D99351: [CSSPGO] Top-down processing order based on full profile..

Addressing Wenlei's comments.

Fri, Mar 26, 12:17 PM · Restricted Project
hoy added inline comments to D99351: [CSSPGO] Top-down processing order based on full profile..
Fri, Mar 26, 12:16 PM · Restricted Project

Thu, Mar 25

hoy accepted D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.
Thu, Mar 25, 10:55 PM · Restricted Project
hoy accepted D99394: [SampleFDO] Do not scale the magic number NOMORE_ICP_MAGICNUM in value profile during profile update..

LGTM, thanks for the fix.

Thu, Mar 25, 10:25 PM · Restricted Project
hoy added inline comments to D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.
Thu, Mar 25, 10:22 PM · Restricted Project
hoy added inline comments to D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.
Thu, Mar 25, 10:02 PM · Restricted Project
hoy added inline comments to D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.
Thu, Mar 25, 5:51 PM · Restricted Project
hoy accepted D99370: [CSSPGO] Minor tweak for inline candidate priority tie breaker.
Thu, Mar 25, 4:53 PM · Restricted Project
hoy added inline comments to D99370: [CSSPGO] Minor tweak for inline candidate priority tie breaker.
Thu, Mar 25, 4:13 PM · Restricted Project
hoy added inline comments to D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.
Thu, Mar 25, 12:31 PM · Restricted Project
hoy updated the summary of D99351: [CSSPGO] Top-down processing order based on full profile..
Thu, Mar 25, 9:15 AM · Restricted Project
hoy updated the summary of D99351: [CSSPGO] Top-down processing order based on full profile..
Thu, Mar 25, 9:15 AM · Restricted Project
hoy updated the summary of D99351: [CSSPGO] Top-down processing order based on full profile..
Thu, Mar 25, 9:14 AM · Restricted Project
hoy requested review of D99351: [CSSPGO] Top-down processing order based on full profile..
Thu, Mar 25, 9:09 AM · Restricted Project

Wed, Mar 24

hoy added inline comments to D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.
Wed, Mar 24, 11:08 AM · Restricted Project

Mon, Mar 22

hoy updated subscribers of D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Mon, Mar 22, 12:06 PM · Restricted Project

Sun, Mar 21

hoy added inline comments to D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Sun, Mar 21, 11:09 PM · Restricted Project

Sat, Mar 20

hoy accepted D98921: [CSSPGO][llvm-profgen] Use profile summary based threshold for context trimming and merging.
Sat, Mar 20, 10:31 PM · Restricted Project
hoy added a comment to D98921: [CSSPGO][llvm-profgen] Use profile summary based threshold for context trimming and merging.

Using PSI looks good, thanks. Can you please add a test for -csprof-merge-cold-context? Otherwise lgtm.

Sat, Mar 20, 4:24 PM · Restricted Project

Fri, Mar 19

hoy added inline comments to D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Fri, Mar 19, 6:35 PM · Restricted Project
hoy added inline comments to D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Fri, Mar 19, 5:47 PM · Restricted Project
hoy added inline comments to D98921: [CSSPGO][llvm-profgen] Use profile summary based threshold for context trimming and merging.
Fri, Mar 19, 12:35 PM · Restricted Project
hoy added inline comments to D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Fri, Mar 19, 10:49 AM · Restricted Project

Thu, Mar 18

hoy committed rGfc1812a0ad75: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and… (authored by hoy).
[UniqueLinkageName] Use consistent checks when mangling symbo linkage name and…
Thu, Mar 18, 10:12 PM
hoy closed D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Thu, Mar 18, 10:12 PM · Restricted Project
hoy updated the diff for D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..

Addresssing David's comment.

Thu, Mar 18, 12:47 PM · Restricted Project
hoy added inline comments to D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name..
Thu, Mar 18, 12:44 PM · Restricted Project
hoy added inline comments to D98835: [SampleFDO] Don't mix up the existing indirect call value profile with the new value profile annotated after inlining..
Thu, Mar 18, 12:01 PM · Restricted Project
hoy accepted D98823: [CSSPGO] Add attribute metadata for context profile.
Thu, Mar 18, 10:33 AM · Restricted Project
hoy added inline comments to D98835: [SampleFDO] Don't mix up the existing indirect call value profile with the new value profile annotated after inlining..
Thu, Mar 18, 10:18 AM · Restricted Project
hoy added a comment to D98835: [SampleFDO] Don't mix up the existing indirect call value profile with the new value profile annotated after inlining..

Thanks for fixing the assert failure!

Thu, Mar 18, 10:16 AM · Restricted Project
hoy added a comment to D98823: [CSSPGO] Add attribute metadata for context profile.

Thanks for working on this. This gets us closer to a global inlining decision making for thinLTO.

Thu, Mar 18, 9:38 AM · Restricted Project

Wed, Mar 17

hoy added a comment to D98481: [Inliner] Do not inline mutual-recursive functions to avoid exponential size growth..

Thanks for the suggestion. I was thinking about capping on function size so that small callees such as getters/setters can always be inlined. What do you think?

Wed, Mar 17, 11:46 AM · Restricted Project