danielcdh (Dehao Chen)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 26 2015, 2:20 PM (103 w, 6 d)

Recent Activity

Tue, Aug 15

danielcdh closed D36778: Merge debug info when hoist then-else code to if..
Tue, Aug 15, 6:56 PM
danielcdh updated the diff for D36778: Merge debug info when hoist then-else code to if..

add positive test

Tue, Aug 15, 6:56 PM
danielcdh created D36778: Merge debug info when hoist then-else code to if..
Tue, Aug 15, 6:08 PM

Sun, Aug 13

danielcdh added a comment to D36655: Move SampleProfileLoader pass before all simplification passes..

Is there a functional test case (i.e., better matching with this change)?

Sun, Aug 13, 3:49 PM
danielcdh updated the diff for D36655: Move SampleProfileLoader pass before all simplification passes..

update

Sun, Aug 13, 2:12 PM
danielcdh added inline comments to D36655: Move SampleProfileLoader pass before all simplification passes..
Sun, Aug 13, 2:12 PM
danielcdh created D36655: Move SampleProfileLoader pass before all simplification passes..
Sun, Aug 13, 1:12 PM

Fri, Aug 11

danielcdh created D36637: Import all inlined indirect call targets for SamplePGO..
Fri, Aug 11, 2:05 PM

Wed, Aug 9

danielcdh closed D36566: Revert part of r310296 to make it really NFC for instrumentation PGO..
Wed, Aug 9, 10:11 PM
danielcdh created D36566: Revert part of r310296 to make it really NFC for instrumentation PGO..
Wed, Aug 9, 9:59 PM

Tue, Aug 8

danielcdh closed D36341: Make ICP uses PSI to check for hotness..
Tue, Aug 8, 2:00 PM

Mon, Aug 7

danielcdh added a comment to D36341: Make ICP uses PSI to check for hotness..

Performance testing on loadtest does not show any difference with this patch with instrumentation based PGO.

Mon, Aug 7, 8:37 PM
danielcdh closed D36333: Move the SampleProfileLoader right after EarlyFPM..
Mon, Aug 7, 1:24 PM
danielcdh added a comment to D36333: Move the SampleProfileLoader right after EarlyFPM..

I think Chandler's concerns for this patch has been addressed. We can continue discussion on the AutoFDO design in a separate thread. Will submit the patch by EOD if no more concerns have been raised.

Mon, Aug 7, 9:23 AM

Sat, Aug 5

danielcdh added a comment to D36341: Make ICP uses PSI to check for hotness..

This change may affect instrumentation PGO. I suspect it will greatly increase count threshold which may negatively affect performance. For instance some not so hot targets have small function body which is always good candidate to inline (after ICP).

Sat, Aug 5, 12:19 PM

Fri, Aug 4

danielcdh updated the diff for D36341: Make ICP uses PSI to check for hotness..

format the source

Fri, Aug 4, 3:50 PM
danielcdh created D36341: Make ICP uses PSI to check for hotness..
Fri, Aug 4, 3:46 PM
danielcdh added a comment to D36333: Move the SampleProfileLoader right after EarlyFPM..

Thanks for the reviews.

Fri, Aug 4, 1:58 PM
danielcdh updated the diff for D36333: Move the SampleProfileLoader right after EarlyFPM..

add cgo2016 paper reference

Fri, Aug 4, 1:57 PM
danielcdh added inline comments to D36333: Move the SampleProfileLoader right after EarlyFPM..
Fri, Aug 4, 1:50 PM
danielcdh updated the diff for D36333: Move the SampleProfileLoader right after EarlyFPM..

update

Fri, Aug 4, 1:49 PM
danielcdh updated the diff for D36333: Move the SampleProfileLoader right after EarlyFPM..

update comment

Fri, Aug 4, 12:59 PM
danielcdh added inline comments to D36333: Move the SampleProfileLoader right after EarlyFPM..
Fri, Aug 4, 12:58 PM
danielcdh updated the diff for D36333: Move the SampleProfileLoader right after EarlyFPM..

update

Fri, Aug 4, 12:38 PM
danielcdh added inline comments to D36333: Move the SampleProfileLoader right after EarlyFPM..
Fri, Aug 4, 12:38 PM
danielcdh created D36333: Move the SampleProfileLoader right after EarlyFPM..
Fri, Aug 4, 11:35 AM
danielcdh closed D36317: Adjust the hotness threshold from 99.9% to 99%..
Fri, Aug 4, 9:21 AM
danielcdh created D36317: Adjust the hotness threshold from 99.9% to 99%..
Fri, Aug 4, 8:41 AM

Thu, Aug 3

danielcdh closed D36025: Do not want to use BFI to get profile count for sample pgo.
Thu, Aug 3, 10:12 AM

Wed, Aug 2

danielcdh updated the diff for D36025: Do not want to use BFI to get profile count for sample pgo.

update test.

Wed, Aug 2, 7:04 PM
danielcdh updated the diff for D36025: Do not want to use BFI to get profile count for sample pgo.

Refactor the code after talking to Easwaran (Thanks!): Move the logic to IsCallsiteCold.

Wed, Aug 2, 6:36 PM
danielcdh updated the diff for D36025: Do not want to use BFI to get profile count for sample pgo.

updated the implementation to include a flag to control whether to return 0 or None if the profile is missing for the entire function. PTAL.

Wed, Aug 2, 6:07 PM
danielcdh closed D36246: Fix the bug when SampleProfileWriter writes out number of callsites..
Wed, Aug 2, 5:09 PM
danielcdh added inline comments to D36246: Fix the bug when SampleProfileWriter writes out number of callsites..
Wed, Aug 2, 4:51 PM
danielcdh created D36246: Fix the bug when SampleProfileWriter writes out number of callsites..
Wed, Aug 2, 3:48 PM
danielcdh updated the diff for D36025: Do not want to use BFI to get profile count for sample pgo.

add comment

Wed, Aug 2, 2:20 PM
danielcdh added a comment to D36025: Do not want to use BFI to get profile count for sample pgo.

ping...

Wed, Aug 2, 9:59 AM

Tue, Aug 1

danielcdh closed D36195: Fix the bug that parseAAPipeline is not invoked in runNewPMPasses in release compiler..
Tue, Aug 1, 8:03 PM
danielcdh added a comment to D36195: Fix the bug that parseAAPipeline is not invoked in runNewPMPasses in release compiler..

I've added the test. Please let me know if you want me to split the patch in 2.

Tue, Aug 1, 6:36 PM
danielcdh updated the diff for D36195: Fix the bug that parseAAPipeline is not invoked in runNewPMPasses in release compiler..

Add test.

Tue, Aug 1, 6:34 PM
danielcdh closed D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..
Tue, Aug 1, 6:29 PM
danielcdh added a comment to D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..

Looks awesome, LGTM.

This does move ICP to pre-cleanup passes for non-ThinLTO builds with sample PGO. I assume you've tested it and this is fine. (It also moves the sample profile loading, but that one seems completely innocuous to me.)

Tue, Aug 1, 5:13 PM
danielcdh added a comment to D36195: Fix the bug that parseAAPipeline is not invoked in runNewPMPasses in release compiler..

Tried to add a test. But either need to have

Tue, Aug 1, 4:31 PM
danielcdh created D36195: Fix the bug that parseAAPipeline is not invoked in runNewPMPasses in release compiler..
Tue, Aug 1, 3:15 PM

Sat, Jul 29

danielcdh updated the diff for D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..

rebase and make instrumentation PGO behavior unchanged.

Sat, Jul 29, 10:32 PM
danielcdh closed D36053: Refactor the build{Module|Function}SimplificationPipeline to expose optimization phase..
Sat, Jul 29, 9:57 PM
danielcdh updated the diff for D36053: Refactor the build{Module|Function}SimplificationPipeline to expose optimization phase..

update. Thanks for the review!

Sat, Jul 29, 8:40 PM
danielcdh updated the diff for D36053: Refactor the build{Module|Function}SimplificationPipeline to expose optimization phase..

update

Sat, Jul 29, 7:58 PM
danielcdh added a comment to D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..

Instr PGO has its own set of simplification passes, but still I think we should keep this patch NFC for instrPGO. If there are performance numbers that justify the InstrPGO pipeline changes, it can be discussed/done in a separate thread. This one should be left as SamplePGO only change.

FWIW, I do agree with you Dehao that we seem to be deing redundant simplification, but as David says here we shouldn't change that behavior in this patch.

The tricky part is that PGOOptions was not passed in the ThinLTO backend before this patch. But after this patch, it will be passed in. However, in buildModuleSimplificationPipeline, it cannot know if this is the ThinLTO backend. As a result, we cannot disable instrumentation/annotation pass in there.

The potential solution could be:

  • add another boolean value in buildModuleSimplificationPipeline to indicate it's ThinLTO backend
  • refactor the code from the beginning of buildModuleSimplificationPipeline to the PGOOpt handling to a separate function, and call it separately in the caller of buildModuleSimplificationPipeline.

I actually think the second option might be interesting, but before we do that I think understanding how much redundancy there really is in the cleanup passes is important. That will help inform how to refactor these into separate components, or if there even *are* separate components. So I largely agree w/ David that something like #1 is the best path forward.

However, rather than adding another boolean value though, I think you should turn the current boolean into an enumeration for ThinLTO phase; "none" by default, and then with beth prelink and postlink values.

Note that I don't think we should call the phase "backend" or "codegen". Those are heavily overloaded in LLVM, so I think pre/post-link is a better terminology to use.

Sat, Jul 29, 6:22 PM
danielcdh created D36053: Refactor the build{Module|Function}SimplificationPipeline to expose optimization phase..
Sat, Jul 29, 6:20 PM
danielcdh added a comment to D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..

Instr PGO has its own set of simplification passes, but still I think we should keep this patch NFC for instrPGO. If there are performance numbers that justify the InstrPGO pipeline changes, it can be discussed/done in a separate thread. This one should be left as SamplePGO only change.

Sat, Jul 29, 4:39 PM
danielcdh added inline comments to D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..
Sat, Jul 29, 4:24 PM
danielcdh created D36052: Update the new PM pipeline to make ICP aware if it is SamplePGO build..
Sat, Jul 29, 3:03 PM

Fri, Jul 28

danielcdh closed D36040: Refine the PGOOpt and SamplePGOSupport handling..
Fri, Jul 28, 9:11 PM
danielcdh updated the diff for D36040: Refine the PGOOpt and SamplePGOSupport handling..

update

Fri, Jul 28, 7:22 PM
danielcdh added inline comments to D36040: Refine the PGOOpt and SamplePGOSupport handling..
Fri, Jul 28, 7:22 PM
danielcdh added inline comments to D36040: Refine the PGOOpt and SamplePGOSupport handling..
Fri, Jul 28, 6:57 PM
danielcdh updated the diff for D36040: Refine the PGOOpt and SamplePGOSupport handling..

update

Fri, Jul 28, 6:57 PM
danielcdh updated the diff for D36040: Refine the PGOOpt and SamplePGOSupport handling..

Integrate Chandler's reviews. Thanks!

Fri, Jul 28, 6:31 PM
danielcdh added inline comments to D36040: Refine the PGOOpt and SamplePGOSupport handling..
Fri, Jul 28, 6:31 PM
danielcdh added a comment to D36040: Refine the PGOOpt and SamplePGOSupport handling..

I think this adds way too much complexity to these predicates. I think we need a much better approach here... Have you looked at alternatives? Is the technique being used to add the descriminators pass just the wrong technique given the interaction with ThinLTO

Fri, Jul 28, 5:47 PM
danielcdh created D36040: Refine the PGOOpt and SamplePGOSupport handling..
Fri, Jul 28, 5:32 PM
danielcdh added a comment to D36025: Do not want to use BFI to get profile count for sample pgo.

If the profile count based on BFI is completely reliable that you are discarding it, then shouldn't BFI->getProfileCount also return 0 for sampled profiling? I think this can end up in surprising behavior.

We only want to do this on callsites, as we still rely on BFI to get other block frequencies.

So, either the BFI based count is usable in sample PGO or not. If it is the latter, I think it should not be used at all. If it is usable (but not very accurate), then there is no harm in using in call.

Fri, Jul 28, 4:52 PM
danielcdh added a comment to D36025: Do not want to use BFI to get profile count for sample pgo.

If the profile count based on BFI is completely reliable that you are discarding it, then shouldn't BFI->getProfileCount also return 0 for sampled profiling? I think this can end up in surprising behavior.

Fri, Jul 28, 4:17 PM
danielcdh added a comment to D36030: [LTO] llvm-lto2: Add option to load sample profile.

Thanks for doing this! I was about to do the same thing as I have a new patch to that relies on this patch for testing.

Fri, Jul 28, 4:10 PM
danielcdh accepted D36030: [LTO] llvm-lto2: Add option to load sample profile.
Fri, Jul 28, 3:58 PM
danielcdh created D36025: Do not want to use BFI to get profile count for sample pgo.
Fri, Jul 28, 3:25 PM
danielcdh closed D35964: Change INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE from 8 to 16..
Fri, Jul 28, 8:01 AM

Thu, Jul 27

danielcdh closed D35966: Changing the default MaxNumPromotions from 2 to 3..
Thu, Jul 27, 6:04 PM
danielcdh closed D35962: Separate the ICP total threshold and remaining threshold..
Thu, Jul 27, 6:04 PM
danielcdh closed D35969: Increase the ImportHotMultiplier to 10.0.
Thu, Jul 27, 6:04 PM
danielcdh added inline comments to D35962: Separate the ICP total threshold and remaining threshold..
Thu, Jul 27, 5:20 PM
danielcdh created D35969: Increase the ImportHotMultiplier to 10.0.
Thu, Jul 27, 5:18 PM
danielcdh created D35966: Changing the default MaxNumPromotions from 2 to 3..
Thu, Jul 27, 4:39 PM
danielcdh created D35964: Change INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE from 8 to 16..
Thu, Jul 27, 4:27 PM
danielcdh created D35962: Separate the ICP total threshold and remaining threshold..
Thu, Jul 27, 4:23 PM
danielcdh added a comment to D35746: Make new PM honor -fdebug-info-for-profiling (clang side).

Thanks for the review!

Thu, Jul 27, 8:30 AM
danielcdh closed D35746: Make new PM honor -fdebug-info-for-profiling (clang side).
Thu, Jul 27, 8:30 AM

Wed, Jul 26

danielcdh updated the diff for D35746: Make new PM honor -fdebug-info-for-profiling (clang side).

update

Wed, Jul 26, 10:14 AM
danielcdh added inline comments to D35746: Make new PM honor -fdebug-info-for-profiling (clang side).
Wed, Jul 26, 10:14 AM
danielcdh added a comment to D35744: Make new PM honor -fdebug-info-for-profiling.

Just FYI, test/Other/new-pm-pgo.ll appears to be failing on several of the buildbots. One example: http://lab.llvm.org:8011/builders/clang-hexagon-elf/builds/10801

Wed, Jul 26, 8:49 AM
danielcdh closed D35744: Make new PM honor -fdebug-info-for-profiling.
Wed, Jul 26, 8:01 AM

Tue, Jul 25

danielcdh updated the diff for D35744: Make new PM honor -fdebug-info-for-profiling.

update

Tue, Jul 25, 10:08 PM
danielcdh updated the diff for D35744: Make new PM honor -fdebug-info-for-profiling.

update flag name and add test.

Tue, Jul 25, 9:27 PM
danielcdh added inline comments to D35744: Make new PM honor -fdebug-info-for-profiling.
Tue, Jul 25, 9:27 PM
danielcdh added a comment to D35807: Add test coverage for new PM PGOOpt handling..

I think the command line flags can be made simpler, but I must not be explaining it well in review. I suggest you go ahead and submit this and I'll immediately send you a follow-up patch that hopefully illustrates what I'm suggesting for handling the flags. And that will let you rebase and continue on other things.

Tue, Jul 25, 6:58 PM
danielcdh updated the diff for D35807: Add test coverage for new PM PGOOpt handling..

update

Tue, Jul 25, 6:38 PM
danielcdh updated the diff for D35746: Make new PM honor -fdebug-info-for-profiling (clang side).

update

Tue, Jul 25, 3:48 PM
danielcdh updated the diff for D35744: Make new PM honor -fdebug-info-for-profiling.

update the option to put it within PGOOPtions. Will add test once https://reviews.llvm.org/D35807 is committed.

Tue, Jul 25, 3:39 PM
danielcdh updated the diff for D35807: Add test coverage for new PM PGOOpt handling..

update

Tue, Jul 25, 3:27 PM
danielcdh added inline comments to D35807: Add test coverage for new PM PGOOpt handling..
Tue, Jul 25, 3:27 PM

Mon, Jul 24

danielcdh updated the diff for D35807: Add test coverage for new PM PGOOpt handling..

address David's comments.

Mon, Jul 24, 11:59 AM
danielcdh created D35807: Add test coverage for new PM PGOOpt handling..
Mon, Jul 24, 10:32 AM

Jul 21 2017

danielcdh added inline comments to D35744: Make new PM honor -fdebug-info-for-profiling.
Jul 21 2017, 4:24 PM
danielcdh created D35746: Make new PM honor -fdebug-info-for-profiling (clang side).
Jul 21 2017, 3:59 PM
danielcdh added a comment to D35744: Make new PM honor -fdebug-info-for-profiling.

Chandler, could you help suggest how I could add unittest for this change? I was trying to find similar test for PGOOpt, but could not find any tests that's related.

Jul 21 2017, 3:57 PM
danielcdh created D35744: Make new PM honor -fdebug-info-for-profiling.
Jul 21 2017, 3:53 PM

Jul 11 2017

danielcdh created D35261: Change GlobalValueSummaryMapTy from std::map to DenseMap..
Jul 11 2017, 8:52 AM

Jul 10 2017

danielcdh closed D35153: Use DenseMap instead std::map for GVSummaryMapTy.
Jul 10 2017, 1:31 PM
danielcdh closed D35148: Use DenseMap instead std::map for GVSummaryMapTy..
Jul 10 2017, 1:13 PM