Page MenuHomePhabricator

davidxl (David Li)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

davidxl added inline comments to D147297: [AutoFDO]Merge called target in body samples.
Fri, Mar 31, 9:54 AM · Restricted Project, Restricted Project

Mon, Mar 20

davidxl added a comment to D146452: [AutoFDO] Use flattened profiles for profile staleness metrics.

Document the new llvm-profdata option in the Commandline guide.

Mon, Mar 20, 10:07 PM · Restricted Project, Restricted Project
davidxl added a reviewer for D146452: [AutoFDO] Use flattened profiles for profile staleness metrics: huangjd.
Mon, Mar 20, 8:55 PM · Restricted Project, Restricted Project

Tue, Mar 14

davidxl accepted D146099: [Pipeline] Remove early InstCombine in ThinLTO post link sample profile pipeline.
Tue, Mar 14, 5:59 PM · Restricted Project, Restricted Project

Tue, Mar 7

davidxl added a comment to D145528: [memprof] Simplify initialized flags..

looks good to me (and less confusing ;) ).

Tue, Mar 7, 4:21 PM · Restricted Project, Restricted Project

Mar 1 2023

davidxl added a comment to D145008: [ControlHeightReduction] Don't combine a "poison" branch.

The value may be poison even if it's not literally poison. The problem seems to be that the condition gets swapped. CHR generates select i1 poison, i1 %arg, i1 false rather than the correct select i1 %arg, i1 poison, i1 false. Either that swapping should not occur (if it's not important to what CHR is doing) or the condition needs to be frozen.

Correct -- kazu@ brought it up before sending the patch as well -- is there existing interface to query for propagated poison?

There is isGuaranteedNotToBeUndefOrPoison(), but here you want to use freeze as a fallback.

The swap is triggered because of the profile data says it is profitable -- while it is itself a bug to deal with, it is independent to this fix.

How can the swap be profitable when flattening control flow into a select? Shouldn't it be independent of order at that point?

Mar 1 2023, 12:46 PM · Restricted Project, Restricted Project
davidxl added a comment to D145008: [ControlHeightReduction] Don't combine a "poison" branch.

The value may be poison even if it's not literally poison. The problem seems to be that the condition gets swapped. CHR generates select i1 poison, i1 %arg, i1 false rather than the correct select i1 %arg, i1 poison, i1 false. Either that swapping should not occur (if it's not important to what CHR is doing) or the condition needs to be frozen.

Mar 1 2023, 10:12 AM · Restricted Project, Restricted Project

Feb 28 2023

davidxl accepted D145008: [ControlHeightReduction] Don't combine a "poison" branch.

lgtm

Feb 28 2023, 2:59 PM · Restricted Project, Restricted Project

Feb 16 2023

davidxl accepted D144220: New SetOperations and unittesting for all SetOperations.
Feb 16 2023, 3:05 PM · Restricted Project, Restricted Project

Feb 6 2023

davidxl edited reviewers for D143460: [CSPGO][CHR] Disable CHR in ThinLTOPostLink mode, added: tejohnson, aeubanks; removed: davidxl.
Feb 6 2023, 9:07 PM · Restricted Project, Restricted Project
davidxl added a comment to D143460: [CSPGO][CHR] Disable CHR in ThinLTOPostLink mode.

There is a related patch by Arthur: https://reviews.llvm.org/D143424. Can that patch solve the problem?

Feb 6 2023, 9:06 PM · Restricted Project, Restricted Project

Jan 20 2023

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

Note the size here is not static code size (it excludes cold code). It is actually a proxy to model the runtime cost due to increased instruction footprint (icache pressure). The multiplier's role is to make the savings and 'size' cost comparable in terms of unit. The cycle name here is also counted at IR level, so not at low level.

I understand that, but it still doesn't make the comparison of different units any better. Introducing a scaling factor is irrelevant, it's just an arbitrary scalar.

Jan 20 2023, 4:01 PM · Restricted Project, Restricted Project
davidxl added a comment to D98213: [InlineCost] Enable the cost benefit analysis on FDO.

I want to resurrect this change that enabled these cost-benefit heuristics originally developed in https://reviews.llvm.org/D92780

There are a number of issues with the original change that I can see, and this has caused all manner of problems for us internally as we moved to the new pass manager (and therefore started using these heuristics).

The main one being that the heuristics seem to completely override all sensible thresholds for code size. The feature caims to compare the performance gains by measuring the cycles saved from a PGO run, and comparing against some threshold. This doesn't make sense since the length of the runs for PGO are arbitrary, and you shouldn't be using them in absolute terms. The only way it makes sense is if you have a very specific workload and the PGO profile run is exactly the same as the real world runs. The original change gave absolutely no justification, formal or intuitive, for the use of the equation to determine profitability. Giving positive runtime results is no justification for completely overriding any reasonable thresholds and potentially impacting code size & compile time by several orders of magnitude as we've seen internally at Apple.

Even worse: D92780 was accepted into mainline LLVM with absolutely no tests, and then enabled for all users in this change. I didn't even see any discussion about tests, or explanation for why they were impractical. We've disabled these heuristics downstream but I absolutely believe that this must be reverted upstream too until it can be re-worked and re-reviewed in detail.

FYI: the cycle savings are normalized by dividing the function entry count, so it is not used in absolute sense.

Right, that's fair enough. See the following comment from the code:

//  CycleSavings      PSI->getOrCompHotCountThreshold()
// -------------- >= -----------------------------------
//       Size              InlineSavingsMultiplier

Cycles are being compared against code size here. How is that metric supposed to be interpreted? What does (cycles/entry)/(bytes) even mean as a unit of measurement? The feature itself seems to be trying to optimize too fine details in machine cycles in a part of the compiler that shouldn't be dealing with such low level specifics.

Jan 20 2023, 3:19 PM · Restricted Project, Restricted Project
davidxl added a comment to D98213: [InlineCost] Enable the cost benefit analysis on FDO.

I want to resurrect this change that enabled these cost-benefit heuristics originally developed in https://reviews.llvm.org/D92780

There are a number of issues with the original change that I can see, and this has caused all manner of problems for us internally as we moved to the new pass manager (and therefore started using these heuristics).

The main one being that the heuristics seem to completely override all sensible thresholds for code size. The feature caims to compare the performance gains by measuring the cycles saved from a PGO run, and comparing against some threshold. This doesn't make sense since the length of the runs for PGO are arbitrary, and you shouldn't be using them in absolute terms. The only way it makes sense is if you have a very specific workload and the PGO profile run is exactly the same as the real world runs. The original change gave absolutely no justification, formal or intuitive, for the use of the equation to determine profitability. Giving positive runtime results is no justification for completely overriding any reasonable thresholds and potentially impacting code size & compile time by several orders of magnitude as we've seen internally at Apple.

Even worse: D92780 was accepted into mainline LLVM with absolutely no tests, and then enabled for all users in this change. I didn't even see any discussion about tests, or explanation for why they were impractical. We've disabled these heuristics downstream but I absolutely believe that this must be reverted upstream too until it can be re-worked and re-reviewed in detail.

Jan 20 2023, 2:39 PM · Restricted Project, Restricted Project

Jan 13 2023

davidxl accepted D141393: [llvm][ir] Purge MD_prof custom accessors.

lgtm

Jan 13 2023, 9:36 PM · Restricted Project, Restricted Project

Jan 12 2023

davidxl added inline comments to D36864: [Profile] backward propagate profile data in jump-threading.
Jan 12 2023, 1:14 PM · Restricted Project

Jan 10 2023

davidxl added a comment to D141393: [llvm][ir] Purge MD_prof custom accessors.

great cleanup overall.

Jan 10 2023, 8:25 PM · Restricted Project, Restricted Project

Jan 3 2023

davidxl added inline comments to D140908: [MemProf] Context disambiguation cloning pass [patch 1a/3].
Jan 3 2023, 10:17 PM · Restricted Project, Restricted Project
davidxl accepted D140741: [llvm-profdata] Remove unnecessary file size check.

The check looks unnecessary or misplaced.

Jan 3 2023, 10:44 AM · Restricted Project, Restricted Project

Dec 29 2022

davidxl added a comment to D140764: [MemProf] Fix inline propagation of memprof metadata.

Is the MIB removal purely for the purpose of reducing memory usage? It seems safe to always keep them.

Dec 29 2022, 1:57 PM · Restricted Project, Restricted Project

Dec 21 2022

davidxl added a comment to D140337: Propagate branchweight through phi values when ExpectedValue is unlikely in LowerExpectIntrinsic.

It might be better to ask for commit access. It is needed for potential follow-ups.

Dec 21 2022, 3:20 PM · Restricted Project, Restricted Project
davidxl accepted D140337: Propagate branchweight through phi values when ExpectedValue is unlikely in LowerExpectIntrinsic.

lgtm

Dec 21 2022, 11:48 AM · Restricted Project, Restricted Project
davidxl added a comment to D140337: Propagate branchweight through phi values when ExpectedValue is unlikely in LowerExpectIntrinsic.

never mind. I misunderstood the usage of this variable. There is no need to adjust.

Dec 21 2022, 11:48 AM · Restricted Project, Restricted Project

Dec 20 2022

davidxl added inline comments to D140337: Propagate branchweight through phi values when ExpectedValue is unlikely in LowerExpectIntrinsic.
Dec 20 2022, 11:14 PM · Restricted Project, Restricted Project
davidxl added a comment to D138602: [WIP] Alwaysinliner time explosion with new pass manager.

It seems that the explosion can happen when the always-inline callee has multiple call edges within the nontrivial SCC. Should the check be more refined?

Dec 20 2022, 10:53 PM · Restricted Project, Restricted Project

Dec 7 2022

davidxl added a comment to D139603: [llvm-profdata] Add option to cap profile output size.

Add a test case for completeness.

Dec 7 2022, 8:54 PM · Restricted Project, Restricted Project

Dec 6 2022

davidxl accepted D139486: [llvm-profdata] Drop profile symbol list during merging AutoFDO profiles..

lgtm

Dec 6 2022, 7:50 PM · Restricted Project, Restricted Project

Nov 29 2022

davidxl accepted D138893: [llvm-profdata] Use flattening sample profile in profile supplementation.

lgtm

Nov 29 2022, 2:48 PM · Restricted Project, Restricted Project
davidxl accepted D138950: [InstCombine] Revert D125845.

thanks for reverting. There is test case available to identify the root cause of regressions.

Nov 29 2022, 1:59 PM · Restricted Project, Restricted Project
davidxl added inline comments to D138893: [llvm-profdata] Use flattening sample profile in profile supplementation.
Nov 29 2022, 1:24 PM · Restricted Project, Restricted Project

Nov 28 2022

davidxl added inline comments to D138893: [llvm-profdata] Use flattening sample profile in profile supplementation.
Nov 28 2022, 9:57 PM · Restricted Project, Restricted Project
davidxl added inline comments to D125845: [InstCombine] Canonicalize GEP of GEP by swapping constant-indexed GEP to the back.
Nov 28 2022, 11:04 AM · Restricted Project, Restricted Project

Nov 22 2022

davidxl added a comment to D138490: [SelectOpt] Don't treat LogicalAnd/LogicalOr as selects.

This looks good. Sotiris may help with a quick benchmarking for the patch.

Nov 22 2022, 10:34 AM · Restricted Project, Restricted Project
davidxl accepted D138333: [CHR] Add a threshold for the code duplication.

lgtm

Nov 22 2022, 10:08 AM · Restricted Project, Restricted Project

Nov 21 2022

davidxl added inline comments to D138333: [CHR] Add a threshold for the code duplication.
Nov 21 2022, 2:14 PM · Restricted Project, Restricted Project

Nov 18 2022

davidxl added inline comments to D138333: [CHR] Add a threshold for the code duplication.
Nov 18 2022, 3:24 PM · Restricted Project, Restricted Project
davidxl added inline comments to D138332: [Passes] Don't add CHR pass for CSIRInstr build in PostLink.
Nov 18 2022, 3:12 PM · Restricted Project, Restricted Project

Nov 6 2022

davidxl accepted D137467: [NFC][BlockPlacement]Add an option to renumber blocks based on function layout order..

lgtm

Nov 6 2022, 9:17 PM · Restricted Project, Restricted Project
davidxl added a comment to D137467: [NFC][BlockPlacement]Add an option to renumber blocks based on function layout order..

I see. The DOT CFG graph dump uses the layout order instead of the MBB number. Another way to fix this is to use the following in the node label: BBName[MBBNum][layout-order], but making the MBB num and layout number consistent under an option is fine to me.

Nov 6 2022, 9:17 PM · Restricted Project, Restricted Project

Nov 4 2022

davidxl added a comment to D137467: [NFC][BlockPlacement]Add an option to renumber blocks based on function layout order..

or show an example how the mapping is made harder without renumbering.

Nov 4 2022, 3:12 PM · Restricted Project, Restricted Project
davidxl added a comment to D137467: [NFC][BlockPlacement]Add an option to renumber blocks based on function layout order..

Does MIR text output show MBB number in the dump?

Nov 4 2022, 3:10 PM · Restricted Project, Restricted Project

Nov 2 2022

davidxl added a comment to D137212: [InstCombine] Simplify chain of GEP with constant indices.

should be

y  = tmp + i + x;
Nov 2 2022, 11:24 AM · Restricted Project, Restricted Project
davidxl added a comment to D137212: [InstCombine] Simplify chain of GEP with constant indices.

This is essentially forward propagation (with multi-use) and is a generally a good way to reduce dependence height. For instance it can be used to enable reassociation and LICM:

Nov 2 2022, 11:22 AM · Restricted Project, Restricted Project
davidxl accepted D137184: [PGO] Add a threshold for number of critical edges in PGO.

lgtm

Nov 2 2022, 9:31 AM · Restricted Project, Restricted Project

Nov 1 2022

davidxl added a comment to D137184: [PGO] Add a threshold for number of critical edges in PGO.

Add a small test case (with small threshold)?

Nov 1 2022, 12:07 PM · Restricted Project, Restricted Project

Oct 27 2022

davidxl added a comment to D135929: [profile] Add binary ids into indexed profiles.

What is the use case for the feature?

Our current coverage pipeline has the following steps:

  1. Read binary-id in each raw profile (.profraw) via using llvm-profdata
  2. Fetch all the binaries with the corresponding binary id
  3. Merge all the raw profiles, and generate an indexed profile (.profdata)
  4. Generate a coverage report via using llvm-cov by providing all the fetched binaries and indexed profile.

We would like to simplify our coverage pipeline by using debuginfod in llvm-cov. When we include binary ids into indexed profiles and start using debuginfod in llvm-cov, our coverage pipeline can look like this:

  1. Merge all the raw profiles, and generate an indexed profile (.profdata)
  2. Generate a coverage report via using llvm-cov by providing only an indexed profile. llvm-cov is going to read all the binary ids from an indexed profile, and fetch the associated binaries from the debuginfod server.
Oct 27 2022, 9:18 AM · Restricted Project, Restricted Project, Restricted Project

Oct 26 2022

davidxl added a comment to D135929: [profile] Add binary ids into indexed profiles.

What is the use case for the feature?

Oct 26 2022, 11:22 AM · Restricted Project, Restricted Project, Restricted Project

Oct 25 2022

davidxl added reviewers for D136698: [SampleFDO] Persist profile staleness metrics into binary: kazu, mtrofin.
Oct 25 2022, 4:04 PM · Restricted Project, Restricted Project
davidxl added a comment to D136698: [SampleFDO] Persist profile staleness metrics into binary.

For incremental build, are the artifacts (build logs) also cached?

Oct 25 2022, 4:03 PM · Restricted Project, Restricted Project
davidxl added a comment to D136698: [SampleFDO] Persist profile staleness metrics into binary.

The use case of the feature is still a little fuzzy to me. Why is the mismatch issue not caught and handled at build time?

Oct 25 2022, 2:01 PM · Restricted Project, Restricted Project
davidxl accepted D136627: [SampleFDO] Compute and report profile staleness metrics.

lgtm

Oct 25 2022, 1:56 PM · Restricted Project, Restricted Project

Oct 24 2022

davidxl added inline comments to D136627: [SampleFDO] Compute and report profile staleness metrics.
Oct 24 2022, 1:13 PM · Restricted Project, Restricted Project
davidxl added inline comments to D136627: [SampleFDO] Compute and report profile staleness metrics.
Oct 24 2022, 12:22 PM · Restricted Project, Restricted Project

Oct 20 2022

davidxl added a comment to D132773: [SLP] Workaround for vectorizing loads with more than one store uses..

Naive question -- can this be done as a post-processing step with proper reordering?

Oct 20 2022, 10:07 AM · Restricted Project, Restricted Project
davidxl added a comment to D132773: [SLP] Workaround for vectorizing loads with more than one store uses..

Vasileios, Abataev's suggestion seems reasonable. Can you try it?

Oct 20 2022, 9:41 AM · Restricted Project, Restricted Project

Oct 13 2022

davidxl added a comment to D129734: [InstCombine] Canonicalize GEP of GEP by swapping constant-indexed GEP to the front.

follow up @davidxl
For the original issue where a chain of 3 or more gep with the first and last being constant indexed cannot be simplified, this patch can handle such case while D125845 can't
Consider the following code

01   b = gep a, const_index
02   use(b)
03   c = gep b, var_index
04   d = gep c, const_index
05   ret d

In this patch line 4 is swapped with line 3, and then it is constant-index folded with line 1 (line 1 is unchanged, and the old line 4 is replaced with gep a with new constant indices, breaking the dependency of b, and result in better codegen.

But if D125845 is used, then line 1 is supposed to be swapped with line 3, but since b has more than 1 use, the swap is inhibited, and nothing can be optimized.

Oct 13 2022, 6:15 PM · Restricted Project, Restricted Project

Oct 11 2022

davidxl added a comment to D135617: [docs] Update compiler-rt/CODE_OWNERS.TXT.

sounds fine to me.

Oct 11 2022, 11:40 AM · Restricted Project, Restricted Project

Oct 7 2022

davidxl added inline comments to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Oct 7 2022, 9:07 AM · Restricted Project, Restricted Project

Oct 5 2022

davidxl added a comment to D135340: [PGO] Make emitted symbols hidden.

Is it due to the mixing instrumented shared libraries built with different compiler and profile versions?

Oct 5 2022, 7:38 PM · Restricted Project, Restricted Project, Restricted Project
davidxl accepted D135317: [InstrProf] Add version into llvm-profdata.

lgtm

Oct 5 2022, 1:56 PM · Restricted Project, Restricted Project, Restricted Project
davidxl added inline comments to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Oct 5 2022, 9:08 AM · Restricted Project, Restricted Project

Oct 4 2022

davidxl added a comment to D135127: [llvm-profdata] Add --output-format option.
  1. the CommandLine documentation file needs to be updated.
  2. what is the use case of splitting text and text-encoding?
Oct 4 2022, 11:19 PM · Restricted Project, Restricted Project

Sep 28 2022

davidxl added a comment to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

It's fine to work around this in the meantime (it is a correctness problem when it comes to mustprogress at least) -- but shouldn't this be done by dropping the loop metadata? I believe that's the general rule for metadata: If a transform cannot safely preserve metadata, this is always resolved in favor of dropping the metadata, not preventing the transform.

Dropping the metadata in this case changes the program semantics (unlike dropping profile data), thus the transformation in this case is unsafe so it should be disabled as this patch does.

I think I am missing what the issue with dropping the loop metadata would be, could you elaborate? If we drop the metadata we should only lose information that may have enabled additional transformations, but shouldn't change the result of the program.

Sep 28 2022, 9:08 AM · Restricted Project, Restricted Project
davidxl added a comment to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

It's fine to work around this in the meantime (it is a correctness problem when it comes to mustprogress at least) -- but shouldn't this be done by dropping the loop metadata? I believe that's the general rule for metadata: If a transform cannot safely preserve metadata, this is always resolved in favor of dropping the metadata, not preventing the transform.

Sep 28 2022, 8:40 AM · Restricted Project, Restricted Project

Sep 27 2022

davidxl accepted D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

Perhaps add a TODO in the comment part for the longer term solution suggested by nikic. Otherwise LGTM

Sep 27 2022, 7:54 PM · Restricted Project, Restricted Project

Sep 25 2022

davidxl accepted D134373: [Analysis] Introduce getStaticBonusApplied (NFC).

lgtm

Sep 25 2022, 11:03 PM · Restricted Project, Restricted Project

Sep 23 2022

davidxl added a comment to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

Jumpthreading 'avoids' the problem with an earlier guard. Since the fix is in a common path, it is ok to just test simplifyCFG.

Sep 23 2022, 9:57 AM · Restricted Project, Restricted Project
davidxl added inline comments to D134373: [Analysis] Introduce getStaticBonusApplied (NFC).
Sep 23 2022, 9:50 AM · Restricted Project, Restricted Project

Sep 22 2022

davidxl added inline comments to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..
Sep 22 2022, 9:24 PM · Restricted Project, Restricted Project
davidxl added a comment to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

Re nikic's comment -- I agree with the statement in general, but I think the infrastructure fix is orthogonal to the bug fix (with the status quo). For what you suggested, we should probably file a bug to track it.

Sep 22 2022, 10:14 AM · Restricted Project, Restricted Project
davidxl added a comment to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

Thanks for the nice graph. It looks like after the simplification, for.cond is used as latch for both the inner and outer loop, but only one MD data is allowed. In this case, I think we should disable the block merge.

Sep 22 2022, 9:46 AM · Restricted Project, Restricted Project

Sep 21 2022

davidxl added inline comments to D134373: [Analysis] Introduce getStaticBonusApplied (NFC).
Sep 21 2022, 10:39 AM · Restricted Project, Restricted Project

Sep 20 2022

davidxl accepted D134125: [IPO] Reorder parameters of InlineFunction (NFC).

lgtm

Sep 20 2022, 8:54 AM · Restricted Project, Restricted Project

Sep 19 2022

davidxl added a reviewer for D134203: [PGO] Avoid assertion on profile generated from code without an exit block: xur.
Sep 19 2022, 12:47 PM · Restricted Project, Restricted Project
davidxl added a comment to D134203: [PGO] Avoid assertion on profile generated from code without an exit block.

This looks like related to infinite loops.

Sep 19 2022, 12:47 PM · Restricted Project, Restricted Project

Sep 18 2022

davidxl added a comment to D134152: [SimplifyCFG][TranformUtils]Do not simplify away a trivial basic block if both this block and at least one of its predecessors are loop latches..

It would be clearer to describe the problem with example CFGs before and after (probably using ascii art) and explain how the meta data is lost during this transformation.

Sep 18 2022, 7:24 PM · Restricted Project, Restricted Project
davidxl accepted D134014: [NFC][SimplifyCFG]Precommit test case to show inner-loop metadata may not be preserved.

lgtm

Sep 18 2022, 6:44 PM · Restricted Project, Restricted Project

Sep 17 2022

davidxl accepted D134117: [Transforms] Merge function attributes within InlineFunction (NFC).

lgtm (The InlineFunction body is getting really big. Probably worth breaking it up in the future).

Sep 17 2022, 10:33 PM · Restricted Project, Restricted Project
davidxl added a comment to D134117: [Transforms] Merge function attributes within InlineFunction (NFC).

Why not adding another parameter to InlineFunction method to specify whether attribute Merge is done?

Sep 17 2022, 7:10 PM · Restricted Project, Restricted Project

Sep 16 2022

davidxl accepted D133999: [SelectOpti] Restrict load sinking.

lgtm

Sep 16 2022, 1:32 PM · Restricted Project, Restricted Project
davidxl added inline comments to D133999: [SelectOpti] Restrict load sinking.
Sep 16 2022, 1:02 PM · Restricted Project, Restricted Project
davidxl added inline comments to D133999: [SelectOpti] Restrict load sinking.
Sep 16 2022, 12:52 PM · Restricted Project, Restricted Project
davidxl added a comment to D133999: [SelectOpti] Restrict load sinking.

Ok -- my confusion was the naming of the SinkPt variable -- it should actually be named 'SI'. I through the sinkpt is the actual bb after select is expanded into branch.

Sep 16 2022, 12:43 PM · Restricted Project, Restricted Project
davidxl added inline comments to D133999: [SelectOpti] Restrict load sinking.
Sep 16 2022, 8:28 AM · Restricted Project, Restricted Project
davidxl added inline comments to D134014: [NFC][SimplifyCFG]Precommit test case to show inner-loop metadata may not be preserved.
Sep 16 2022, 12:10 AM · Restricted Project, Restricted Project

Sep 15 2022

davidxl added inline comments to D133999: [SelectOpti] Restrict load sinking.
Sep 15 2022, 9:19 PM · Restricted Project, Restricted Project

Sep 13 2022

davidxl accepted D133777: [SelectOpti] Fix lifetime intrinsic bug.

It is unclear that the more precise life time tracking can make a big difference given the code region (post transformation) is small, so this handling LGTM.

Sep 13 2022, 11:23 AM · Restricted Project, Restricted Project
davidxl added inline comments to D133777: [SelectOpti] Fix lifetime intrinsic bug.
Sep 13 2022, 9:25 AM · Restricted Project, Restricted Project

Sep 8 2022

davidxl added a comment to D129734: [InstCombine] Canonicalize GEP of GEP by swapping constant-indexed GEP to the front.

The example provided shows that there is a missing opportunity for CSE (of address computation of Vec[i]). The decision on GEP ordering should probably better be based on more general reassociation analysis for enabling more CSE/LICM.

Sep 8 2022, 11:55 AM · Restricted Project, Restricted Project
davidxl added a comment to D128302: [AArch64][CostModel] Detects that {Extract,Insert}Element at lane 0 have the same cost as other lanes for real instructions that operates on integer types .

The patch looks reasonable to me. Check if @fhahn @RKSimon have further comments.

Sep 8 2022, 11:14 AM · Restricted Project, Restricted Project

Sep 6 2022

davidxl added inline comments to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.
Sep 6 2022, 9:05 AM · Restricted Project, Restricted Project

Sep 1 2022

davidxl added a comment to D133121: [PGO] Annotate branch_weight for invoke instruction.

Can you simplify the test case (e.g. stripping the debug info)?

Sep 1 2022, 9:58 AM · Restricted Project, Restricted Project

Aug 31 2022

davidxl accepted D133040: [PGO] Support PGO annotation of CallBrInst.

lgtm

Aug 31 2022, 12:12 PM · Restricted Project, Restricted Project

Aug 29 2022

davidxl accepted D132601: [llvm-profdata] Improve profile supplementation.

lgtm

Aug 29 2022, 11:44 AM · Restricted Project, Restricted Project

Aug 26 2022

davidxl added a comment to D131287: Fix branch weight in FoldCondBranchOnValueKnownInPredecessor pass in SimplifyCFG.

I did not look at the patch in details. At a high level, it seems that the pass itself does not use profile data to make decisions, so BPI/BFI is really not needed. Instead, directly updating the profile meta data as transformation happens is reasonable (unless BFI provides updating APIs for cases like this, which is not the case).

Aug 26 2022, 11:17 PM · Restricted Project, Restricted Project
davidxl added inline comments to D132601: [llvm-profdata] Improve profile supplementation.
Aug 26 2022, 10:58 PM · Restricted Project, Restricted Project
davidxl added a comment to D132601: [llvm-profdata] Improve profile supplementation.

emit an error for merging profiles of different kind is ok.

Aug 26 2022, 10:36 AM · Restricted Project, Restricted Project
davidxl added inline comments to D132601: [llvm-profdata] Improve profile supplementation.
Aug 26 2022, 8:42 AM · Restricted Project, Restricted Project

Aug 25 2022

davidxl accepted D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation.

lgtm

Aug 25 2022, 2:33 PM · Restricted Project, Restricted Project, Restricted Project
davidxl added inline comments to D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation.
Aug 25 2022, 10:50 AM · Restricted Project, Restricted Project, Restricted Project