update tests because API change from main

Cleanup unit test

@snehasish Please re-review since I introduced a major change to the code.

use llvm::unittest::TempFile instead

cleanup

Refactored to fix bugs on non linux platforms

Update unit test, previously may crash on mac OS

Updated unit test

Updated unit test

Updating D141446: Fix to D139603(reverted) - moved size check to unit test so that it is cross-platform

Updating D141446: Fix to D139603(reverted) - moved size check to unit test so that it is cross-platform

In D139603#4038245, @dyung wrote:

@huangjd the test you added seems to be failing on Windows. Can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/216/builds/15534

Clarified comments

Refactor: moved implementation to llvm lib so that it can be used by other tools

Add comment clarifying reset() usage

The check also looks suspicious by casting size_t to uint64_t. If size_t is 64 bit the cast has no effect, and if size_t is 32 bit then the condition is always false, so I don't understand what's the purpose of this check

Updating D139603: [llvm-profdata] Add option to cap profile output size

In D140741#4018895, @snehasish wrote:

Some filesystems have a 4GB limit on file size (notably FAT32). Given that llvm-profdata as a tool is built for multiple platforms, I don't think we should remove this check.

As for down sampling, having a sample count of 0 vs not having a sample means differently to the compiler, so that may change the branch basic block placement on hot functions, not sure if good idea.

The biggest challenge to compute the number of functions accurately is the compression in extbinary, because the compressed size is non-linear to the original size. Since profile samples and function names are written to different sections (and in CS profile the names are split into two sections and samples can also be split into two sections), there is no way to predict ahead the offset between them. Based on use cases, the current heuristic is under estimating how many functions to prune (and the last iteration typically converges to pruning 1 function) so it's unlikely to remove too many functions. (Note: Also tried using cubic equation for heuristic but that will remove too many functions, so the optimal heuristic is between O(n^2) and O(n^3))

Added output size check

Refactored code structure
Use a string buffer to rewrite files
Added API for potential new strategy to reduce profile size

https://discourse.llvm.org/t/rfc-add-option-to-limit-llvm-profdata-profile-output-size/67036 for RFC

I would need some clarification on inbounds keyword. When an inbounds GEP of GEP is being transformed, what kind of transformation and conditions keep the new GEP inbounds? For example GEP inbounds (GEP inbounds P a) b is equivalent to GEP inbounds (GEP inbounds P b) a if and only if a and b have the same sign. Are there other algebraically valid transformations? This actually does affect D137212 since it is swapping constant indexed GEP. Maybe I misunderstood what inbounds implies? I noticed arbitrary pointer arithmetic expression in C generates inbounds GEP even the pointer is clearly not pointing to any allocated object

D138950 to revert this patch. Looks like reverting it will not interfere with D137212

Could someone reproduce the test case (and benchmarking results) showing regression?

Merged after adding baseline

See D137664 for baseline

merge with main

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

Consider the following practical example

struct Vec {
    float  x, y, z;
};

It is more of a precaution, but I did come across very large afdo profiles.

@efriedma This is a limitation of the current patch that it could only handle the case with arguments in matching order. After some investigation I found that the current function call lowering pass cannot handle in-place stack argument shuffling at all, as it doesn't take read/write dependency into account. It would require an extensive refactoring to completely fix it.

Added comments

Restructured logic to determine isSibcall

Updated comments

In D129734#3650863, @nikic wrote:

I think this will do worse in practice, because it breaks GVN. Imagine you have accesses like ary[i].x, ary[i].y, ary[i].z represented with two GEPs. If constants are canonicalized to the end, then the ary[i] GEP is the same all three times and can be CSEd/GVNd. If the constants are canonicalized to the start, then these all become distinct GEPs on different bases.

• Remove now irrelevant test

Note: this is a NEW patch and the implementation is different from D125845.