Page MenuHomePhabricator

davidxl (David Li)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 13 2015, 9:48 AM (312 w, 5 d)

Recent Activity

Mon, Mar 29

davidxl added inline comments to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).
Mon, Mar 29, 11:39 AM · Restricted Project
davidxl added inline comments to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).
Mon, Mar 29, 10:38 AM · Restricted Project

Thu, Mar 25

davidxl accepted D99394: [SampleFDO] Do not scale the magic number NOMORE_ICP_MAGICNUM in value profile during profile update..

lgtm

Thu, Mar 25, 7:56 PM · Restricted Project
davidxl updated subscribers of D98213: [InlineCost] Enable the cost benefit analysis on FDO.

Looks like div by zero error.

Thu, Mar 25, 2:20 PM · Restricted Project

Wed, Mar 24

davidxl added a comment to D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner.

ThinLTO is known to have issues related to profile update (cross module), so we were thinking something similar in ThinLink phase. One of the issues is that the pre-inlining needs to make similar decisions as the compiler. How well is the preinliner doing in this regard?

Wed, Mar 24, 8:22 PM · Restricted Project
davidxl accepted D99302: [InlineCost] Make cost-benefit decision explicit.

lgtm

Wed, Mar 24, 3:46 PM · Restricted Project
davidxl added inline comments to D99302: [InlineCost] Make cost-benefit decision explicit.
Wed, Mar 24, 3:42 PM · Restricted Project
davidxl added inline comments to D99302: [InlineCost] Make cost-benefit decision explicit.
Wed, Mar 24, 3:21 PM · Restricted Project
davidxl added inline comments to D99302: [InlineCost] Make cost-benefit decision explicit.
Wed, Mar 24, 2:59 PM · Restricted Project

Tue, Mar 23

davidxl added a comment to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).

Regarding the staging of the patch, here is the suggestion:

Tue, Mar 23, 1:19 PM · Restricted Project

Mon, Mar 22

davidxl added inline comments to D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO).
Mon, Mar 22, 8:57 PM · Restricted Project

Fri, Mar 19

davidxl accepted D98982: Sync InstrProfData.inc between llvm and compiler-rt.

lgtm

Fri, Mar 19, 3:25 PM · Restricted Project

Thu, Mar 18

davidxl accepted D98835: [SampleFDO] Don't mix up the existing indirect call value profile with the new value profile annotated after inlining..

lgtm

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

Wed, Mar 17

davidxl added inline comments to D98835: [SampleFDO] Don't mix up the existing indirect call value profile with the new value profile annotated after inlining..
Wed, Mar 17, 8:19 PM · Restricted Project
davidxl updated subscribers of D98481: [Inliner] Do not inline mutual-recursive functions to avoid exponential size growth..

that is fine if it does not add visible compile time overhead (e.g, to
compute and track size).

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

Tue, Mar 16

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

Hiroshi is working on the implementation, and I expect upstreaming happening soon. In a nutshell, the working set analysis is based on building interprocedural loop graph (ILG) and working set propagation.

Tue, Mar 16, 7:23 PM · Restricted Project
davidxl added a comment to D98481: [Inliner] Do not inline mutual-recursive functions to avoid exponential size growth..

Having cap on the target context (where a site is inlined into) is useful for performance purpose but it is based on working set analysis which we are working on. This can be 'overloaded' to control compile time growth too.

Tue, Mar 16, 5:00 PM · Restricted Project

Mon, Mar 15

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

Without guarding the caller size, this (over topdown inlining) can also potentially happen without recursion.

Mon, Mar 15, 11:56 PM · Restricted Project
davidxl added a reviewer for D98481: [Inliner] Do not inline mutual-recursive functions to avoid exponential size growth.: kazu.
Mon, Mar 15, 11:02 PM · Restricted Project

Mar 11 2021

davidxl added a comment to D98213: [InlineCost] Enable the cost benefit analysis on FDO.

Right . We (snehasishk) have plan to tune MFS for AutoFDO. Once that is done, the inline cost analysis for AFDO can be revisited.

Mar 11 2021, 4:09 PM · Restricted Project
davidxl added a comment to D98213: [InlineCost] Enable the cost benefit analysis on FDO.

It is probably related to MFS -- machine function splitting is not yet well tuned for AutoFDO, and the cost/benefit analysis needs to sync up with that to that for estimating cold size properlly.

Mar 11 2021, 3:04 PM · Restricted Project
davidxl accepted D98213: [InlineCost] Enable the cost benefit analysis on FDO.

lgtm

Mar 11 2021, 2:24 PM · Restricted Project

Mar 10 2021

davidxl accepted D97592: [PGO] Fix two issues in PGOMemOPSizeOpt..

lgtm

Mar 10 2021, 11:34 AM · Restricted Project, Restricted Project

Mar 9 2021

davidxl added a comment to D98061: [InstrProfiling] Generate runtime hook for ELF platforms.

thanks for the background. This patch looks good at higher level. Vedant can help detailed review.

Mar 9 2021, 11:07 PM · Restricted Project, Restricted Project
davidxl added a comment to D98061: [InstrProfiling] Generate runtime hook for ELF platforms.

Is the case when there is no counters very rare? And for those cases, how much overhead the runtime hook can incur? I assume it is small compared with actual instrumentation?

Mar 9 2021, 6:28 PM · Restricted Project, Restricted Project

Mar 8 2021

davidxl added inline comments to D98213: [InlineCost] Enable the cost benefit analysis on FDO.
Mar 8 2021, 3:12 PM · Restricted Project

Mar 3 2021

davidxl added inline comments to D97350: [SampleFDO] Another fix to prevent repeated indirect call promotion in sample loader pass.
Mar 3 2021, 4:26 PM · Restricted Project
davidxl added inline comments to D97592: [PGO] Fix two issues in PGOMemOPSizeOpt..
Mar 3 2021, 4:03 PM · Restricted Project, Restricted Project

Mar 1 2021

davidxl accepted D97649: [InstrProfiling] Place __llvm_prf_vnodes and __llvm_prf_names in llvm.used on ELF.

LGTM (the a.out file is probably accidentally added).

Mar 1 2021, 11:18 AM · Restricted Project, Restricted Project
davidxl added inline comments to D97649: [InstrProfiling] Place __llvm_prf_vnodes and __llvm_prf_names in llvm.used on ELF.
Mar 1 2021, 10:50 AM · Restricted Project, Restricted Project
davidxl added inline comments to D97649: [InstrProfiling] Place __llvm_prf_vnodes and __llvm_prf_names in llvm.used on ELF.
Mar 1 2021, 9:08 AM · Restricted Project, Restricted Project
davidxl accepted D97697: [test/profile] Add test coverage for __llvm_profile_write_buffer_internal.

lgtm

Mar 1 2021, 9:04 AM · Restricted Project

Feb 28 2021

davidxl added a comment to D97592: [PGO] Fix two issues in PGOMemOPSizeOpt..

Perhaps add a test case showing that up to 3 ranges can be promoted?

Feb 28 2021, 6:29 PM · Restricted Project, Restricted Project
davidxl accepted D97648: [profile] Delete zero-size dummy sections.

lgtm

Feb 28 2021, 6:25 PM · Restricted Project
davidxl added a comment to D97645: [profile] Delete unused __llvm_profile_write_buffer_internal.

not sure if one is used by downstream users -- on the other hand if is used, the name of the API should be changed.

Feb 28 2021, 4:15 PM · Restricted Project

Feb 27 2021

davidxl added inline comments to D97623: [SampleFDO] Add a cutoff flag to control how many symbols will be included into profile symbol list..
Feb 27 2021, 10:50 PM · Restricted Project

Feb 26 2021

davidxl accepted D97585: [InstrProfiling] Use llvm.compiler.used instead of llvm.used for ELF.

lgtm for ELF. Anything to do for COFF?

Feb 26 2021, 4:01 PM · Restricted Project
davidxl added a comment to D97585: [InstrProfiling] Use llvm.compiler.used instead of llvm.used for ELF.

So to summarize, this patch is to enable compiler passes to respect comdat semantics while still enabling linker to do GC (after using GNU_RETAIN)?

Feb 26 2021, 3:46 PM · Restricted Project

Feb 23 2021

davidxl added a comment to D97350: [SampleFDO] Another fix to prevent repeated indirect call promotion in sample loader pass.

The logic seems complicated. Is it better to use (uint64)-1 as the special value to mark promoted candidates?
( I posted this comment on the wrong thread).

Feb 23 2021, 7:44 PM · Restricted Project
davidxl added a comment to rG5fb65c02ca5e: [SampleFDO] Stop repeated indirect call promotion for the same target..

The logic seems complicated. Is it better to use (uint64)-1 as the special value to mark promoted candidates?

Feb 23 2021, 7:31 PM
davidxl accepted D97336: [InstrProfiling] Use nobits for __llvm_prf_cnts with binutils >=2.28.

lgtm

Feb 23 2021, 4:37 PM · Restricted Project
davidxl updated subscribers of D97110: [InstrProfiling] Use nobits as __llvm_prf_cnts section type in ELF.

Kazu has seen this error

Feb 23 2021, 1:52 PM · Restricted Project

Feb 20 2021

davidxl accepted D97110: [InstrProfiling] Use nobits as __llvm_prf_cnts section type in ELF.

lgtm

Feb 20 2021, 9:40 AM · Restricted Project

Feb 19 2021

davidxl accepted D96919: [clang] Emit type metadata on available_externally vtables for WPD.

lgtm

Feb 19 2021, 11:38 AM · Restricted Project
davidxl added inline comments to D96919: [clang] Emit type metadata on available_externally vtables for WPD.
Feb 19 2021, 11:01 AM · Restricted Project
davidxl added inline comments to D96919: [clang] Emit type metadata on available_externally vtables for WPD.
Feb 19 2021, 10:30 AM · Restricted Project
davidxl added a comment to D96919: [clang] Emit type metadata on available_externally vtables for WPD.

So in this case, the virtual call in user code is resolved to a strong definition in the shared library even though user code also has a derived class defined? In other words, the library provides another overrider definition as the final overrider?

Feb 19 2021, 10:16 AM · Restricted Project

Feb 18 2021

davidxl added a comment to D96757: [InstrProfiling] Use ELF section groups for counters, data and values.

llvm_prf_names are cross function for the purpose of compression. This section is not loadable, so it is does not really matter that much.

Feb 18 2021, 9:48 AM · Restricted Project, Restricted Project

Feb 17 2021

davidxl accepted D96936: [profile] Make {__start_,__stop_}__llvm_prf_* symbols undefined weak.

lgtm

Feb 17 2021, 11:26 PM · Restricted Project
davidxl added a comment to D96936: [profile] Make {__start_,__stop_}__llvm_prf_* symbols undefined weak.

Does it work for other linkers with this change?

Feb 17 2021, 11:13 PM · Restricted Project
davidxl accepted D96902: [profile] Add __attribute__((used)) to zero size dummy sections.

lgtm

Feb 17 2021, 2:38 PM · Restricted Project
davidxl added a comment to D95969: [WPD] Add an optional checking mode for debugging devirtualization.

thanks for the update. lgtm

Feb 17 2021, 12:45 PM · Restricted Project
davidxl added a comment to D96757: [InstrProfiling] Use ELF section groups for counters, data and values.

Nice reduction. Is the total .o size quoted from instrumentation build (with --gc-sections)?

Feb 17 2021, 8:23 AM · Restricted Project, Restricted Project

Feb 16 2021

davidxl added a comment to D96757: [InstrProfiling] Use ELF section groups for counters, data and values.

Good point about the section header. The objective of the patch is to reduce binary image size (loadable sections) though the object size may increase. It would be good to know the object size overhead because some of our large binaries may get hit during link time (of instr binary).

Feb 16 2021, 4:54 PM · Restricted Project, Restricted Project
davidxl accepted D96757: [InstrProfiling] Use ELF section groups for counters, data and values.

Looks good to me. Wait for maskray's comments.

Feb 16 2021, 4:38 PM · Restricted Project, Restricted Project
davidxl added a comment to D96757: [InstrProfiling] Use ELF section groups for counters, data and values.

Can this be handled by other linkers (gold, bfd)?

Feb 16 2021, 3:25 PM · Restricted Project, Restricted Project
davidxl added inline comments to D96806: [SampleFDO] Stop repeated indirect call promotion for the same target.
Feb 16 2021, 2:57 PM · Restricted Project
davidxl accepted D95969: [WPD] Add an optional checking mode for debugging devirtualization.

lgtm

Feb 16 2021, 2:30 PM · Restricted Project
davidxl added inline comments to D96757: [InstrProfiling] Use ELF section groups for counters, data and values.
Feb 16 2021, 11:36 AM · Restricted Project, Restricted Project

Feb 12 2021

davidxl added inline comments to D95969: [WPD] Add an optional checking mode for debugging devirtualization.
Feb 12 2021, 9:51 AM · Restricted Project

Feb 9 2021

davidxl added a comment to D95969: [WPD] Add an optional checking mode for debugging devirtualization.

I think this is probably worth a user level option as well.

Feb 9 2021, 11:21 AM · Restricted Project

Feb 8 2021

davidxl accepted D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

lgtm. Please wait for other reviewer's feedback too.

Feb 8 2021, 11:13 AM · Restricted Project

Feb 6 2021

davidxl added a comment to D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

no it is not noise -- the questions you raised are very good :)

Feb 6 2021, 2:53 PM · Restricted Project
davidxl updated subscribers of D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

I think what Rong means is that

  1. moving code to a header is for code sharing between IR and codeGenPass
  2. Wei's idea of avoiding the header move can lead to circular dependencies.
Feb 6 2021, 2:24 PM · Restricted Project

Feb 4 2021

davidxl added inline comments to D92074: [llvm-profdata] Emit Error when Invalid MemOpSize Section is Created by llvm-profdata.
Feb 4 2021, 12:46 PM · Restricted Project

Feb 2 2021

davidxl added a comment to D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

Good to know that SampleCoverageTracker can remain as it is (not templatized). Is it possible to not move it into the header? There are a few advantages:

  1. reduce the size of the patch and merge churns;
  2. a little better in compile time as the functions don't need to be built when building FlowSensitiveSampleLoader.
Feb 2 2021, 6:55 PM · Restricted Project
davidxl added a comment to D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

question: it seems SampleCoverageTracker implementation is independent of IR/MIR level. Do we need to templatize it ?

Feb 2 2021, 1:41 PM · Restricted Project
davidxl added a comment to D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

Is the intention to move them (inline bodies) into the header? For those functions that are not, leave them as being defined out of line. Otherwise moving them into class body is a preparation for the next step (moving to header).

Feb 2 2021, 12:40 PM · Restricted Project
davidxl added a comment to D95832: [SampleFDO][NFC] Refactor SampleProfileLoad to reuse the code in CodeGen.

Instead of having inlining keyword, it is better to move the out of line definitions as inline bodies in the class.

Feb 2 2021, 11:43 AM · Restricted Project

Feb 1 2021

davidxl added a comment to D95823: [SampleFDO][NFC] Detach SampleProfileLoader from SampleCoverageTracker.

Why making callSiteIsHot a file static func?

Feb 1 2021, 2:51 PM · Restricted Project
davidxl added a comment to D95698: [SampleFDO][NFC] Move the core implementation of Sample Profile Loader to a template.

This will make downstream integration/merging and the review process easier.

Feb 1 2021, 10:24 AM · Restricted Project
davidxl added a comment to D95698: [SampleFDO][NFC] Move the core implementation of Sample Profile Loader to a template.

The diff is quite large and it is better to be broken down into smaller pieces in stages.

Feb 1 2021, 10:23 AM · Restricted Project

Jan 26 2021

davidxl accepted D95495: Emit metadata if there is a profile hash mismatch.

lgtm

Jan 26 2021, 10:10 PM · Restricted Project
davidxl added inline comments to D95495: Emit metadata if there is a profile hash mismatch.
Jan 26 2021, 9:28 PM · Restricted Project
davidxl added inline comments to D95495: Emit metadata if there is a profile hash mismatch.
Jan 26 2021, 8:16 PM · Restricted Project

Jan 25 2021

davidxl accepted D94820: Support for instrumenting only selected files or functions.

my bad -- I missed the test case. LGTM

Jan 25 2021, 2:55 PM · Restricted Project, Restricted Project
davidxl added inline comments to D94820: Support for instrumenting only selected files or functions.
Jan 25 2021, 2:00 PM · Restricted Project, Restricted Project
davidxl added inline comments to D94820: Support for instrumenting only selected files or functions.
Jan 25 2021, 12:21 PM · Restricted Project, Restricted Project

Jan 22 2021

davidxl accepted D95269: [SampleFDO] Report error when reading a bad/incompatible profile instead of turning off profile use silently..

lgtm

Jan 22 2021, 4:39 PM · Restricted Project
davidxl added a comment to D95269: [SampleFDO] Report error when reading a bad/incompatible profile instead of turning off profile use silently..

Should it be warning or error?

Jan 22 2021, 3:52 PM · Restricted Project
davidxl added a reviewer for D95209: [CodeGen] Port Machine Function Splitter from ELF to COFF: snehasish.
Jan 22 2021, 10:18 AM · Restricted Project

Jan 20 2021

davidxl added inline comments to D94820: Support for instrumenting only selected files or functions.
Jan 20 2021, 5:25 PM · Restricted Project, Restricted Project
davidxl added a reviewer for D95079: [NFC] Move ImportedFunctionsInliningStatistics to Analysis: dblaikie.
Jan 20 2021, 12:55 PM · Restricted Project

Jan 19 2021

davidxl added a comment to D94982: [NPM][Inliner] Factor ImportedFunctionStats in the InlineAdvisor.

Do you have an example showing the non-determinsticness ?

Jan 19 2021, 1:45 PM · Restricted Project
davidxl added inline comments to D94820: Support for instrumenting only selected files or functions.
Jan 19 2021, 1:22 PM · Restricted Project, Restricted Project

Jan 14 2021

davidxl added a comment to D92074: [llvm-profdata] Emit Error when Invalid MemOpSize Section is Created by llvm-profdata.

I agree with Rong here -- the semantic related error checking belongs to the consumer/client, not in the tool.

Jan 14 2021, 10:56 AM · Restricted Project

Jan 13 2021

davidxl added inline comments to D94613: [NFC] Rename ThinLTOPhase to PhaseInAllLTO and move it from PassBuilder.h to Pass.h.
Jan 13 2021, 11:49 AM · Restricted Project
davidxl accepted D94436: [X86] Add the FSRM feature (Fast Short Rep Mov) to Zen3..

lgtm

Jan 13 2021, 11:46 AM · Restricted Project

Jan 8 2021

davidxl added a comment to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

Should we get rid of DefaultInlineAdvice and fold it into the base class InlineAdvice? 'DefaultInlineAdvice' implies it is an advice provided by the default advisor which no longer is true.

Jan 8 2021, 7:31 PM · Restricted Project, Restricted Project
davidxl added a comment to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

You are right -- the text format is properly implemented.

Jan 8 2021, 6:49 PM · Restricted Project, Restricted Project
davidxl added a comment to D94333: [Inliner] Change inline remark format and update ReplayInlineAdvisor to use it.

The proposal in the patch is to use line:col.discriminator (which is reasonable), but the implementation uses line:col:discriminator -- please update the patch to be consistent.

Jan 8 2021, 1:56 PM · Restricted Project, Restricted Project

Jan 6 2021

davidxl accepted D94199: [PGO][PGSO] Let unroll hints take precedence over PGSO..

lgtm

Jan 6 2021, 2:50 PM · Restricted Project

Jan 5 2021

davidxl added a comment to D94001: [CSSPGO] Call site prioritized inlining for sample PGO.

A few high level questions:

Jan 5 2021, 9:12 AM · Restricted Project
davidxl accepted D93658: [Inliner] Compute the full cost for the cost benefit analsysis.

lgtm

Jan 5 2021, 8:52 AM · Restricted Project

Dec 17 2020

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

lgtm

Dec 17 2020, 1:02 PM · Restricted Project, Restricted Project

Dec 16 2020

davidxl added inline comments to D92493: [IR] Add hot to function attributes and use hot/cold attribute in function section prefix/suffix.
Dec 16 2020, 10:14 AM · Restricted Project, Restricted Project
davidxl added a comment to D92780: [InlineCost] Implement cost-benefit-based inliner.

I suggest also dumping some opt remarks under --pass-remarks-analysis. This also allows you to write test cases to cover various types of savings.

Dec 16 2020, 10:02 AM · Restricted Project

Dec 11 2020

davidxl added inline comments to D92780: [InlineCost] Implement cost-benefit-based inliner.
Dec 11 2020, 11:13 AM · Restricted Project

Dec 10 2020

davidxl accepted D92882: [MBP] Prevent rotating a chain contains entry block.

thanks for working on the test -- good to add later if possible. LGTM after addressing snehasish's comment.

Dec 10 2020, 6:21 PM · Restricted Project
davidxl accepted D92669: [PGO] Adjust -vp-counters-per-site..

lgtm

Dec 10 2020, 10:01 AM · Restricted Project