- User Since
- Aug 20 2015, 4:19 PM (395 w, 3 d)
Thu, Mar 9
Wed, Mar 8
Integrated review comments/suggestions from Wenlei and Hongtao:
(1) remove BBSize from hash
(2) use less expensive hash function
(3) better names for getSubprogramLinkageName()
The performance for removing BBSize from discriminator hash is out: it shows a slightly gain on performance, like ~0.2% vs with BBsize in the hash. I will remove BBSize from the hash.
Mon, Mar 6
Integrated Hongtao's review suggestion.
Thanks Hongtao for the reviews and comments. I'll update the patch shortly.
Thu, Mar 2
Wed, Mar 1
Yes. This patch is obsolete now.
Mon, Feb 27
Fri, Feb 24
Feb 16 2023
I did a perf test and I think this is fine. LGTM.
Feb 6 2023
I think this is the reasonable place for CHR.
With this we will do a lot less CHR, we need to run a performance test to ensure there is no regression.
Yes. D143424 is actually what I wanted to do in the following up patch. We need to do some performance test though.
Dec 15 2022
I have some high level questions:
(1) have you considered removing samples with smaller values across the program-- it's like downsampling. Comparing to removing functions with smaller total counts, I think that results in a more consistent profile.
(2) if we choose to remove function, and you sort the function with total count, should we find the exactly place to cut to satisfy the size limit? In theory it should as the profile organized in unit of function. You can keep writing to the buffer until it reaches the limits. Of cause there are some section data for extbinary and summary, but they should be able to compute. Using heuristic to guess the function to remove and doing it iteratively does not seem to be appealing here.
Dec 7 2022
I kind of agree on your point. I also think ModuleOPtimization pipeline
fits better for this pass. This pass is very aggressive in terms of control
flow. In my cases it duplicated the whole function.
Dec 2 2022
Add comments in the tests suggested by Teresa.
Nov 30 2022
Followed Reid's suggestion to explicitly check the IR after the pass.
Nov 29 2022
Nov 28 2022
Removed lambda updateSampleMap() -- it was from the early version and now it was no longer needed as we just had one caller.
Addressed David's review comments.
Simplified the patch a bit by refactoring the existing interface.
Nov 22 2022
Nov 21 2022
Nov 18 2022
Nov 2 2022
Nov 1 2022
Add a test case, per David's suggestion.
Oct 23 2022
Oct 21 2022
Oct 20 2022
Oct 12 2022
Looks good to me.
Oct 7 2022
Oct 6 2022
Sep 21 2022
Sep 19 2022
This is an interesting example. When we design this, we assume there is at least one exit node in the function. And we create fake edges from exit node(s). This way we create a close graph.
I'm fine with change. But can you add a warning message here? I would be helpful to know something is wrong here.
Sep 7 2022
Sep 6 2022
Integrated Hongtao's suggestion.
Sep 5 2022
Integrated with review comments.
Sep 1 2022
Aug 31 2022
I'm fine with this change.
Aug 29 2022
I agreed with Teresa: adding an optional string is better than using a separated metadata here.
Integrated David's review comment.
For the merge profile merge, also skip the value profile merge if the count is pseudo kind.
Aug 26 2022
Integrated David's most recent review comments.
Integrated Sriraman's suggestion.
This is more complex change that hide the pseudo count values in InstroProfRecord.
Needs to touch more files. But I think this improves the original patch.
Aug 25 2022
Fixed unneeded &
This patches adds the support for mapping to warm function, just following the existing practice (where it maps to hot functions).
The constant of -1 is already in llvm.
Integrated David's review commends.