- User Since
- Apr 13 2015, 9:48 AM (219 w, 1 d)
The refactoring part looks ok. +vsk to double check.
Fri, Jun 21
Is there a bug reference somewhere?
Thu, Jun 13
llvm-cov's user manual probably also needs some update.
is thin archive supported?
Tue, Jun 11
Mon, Jun 10
This can be handled this way:
Warning. With static profiling, the layout strategy based on the 'precise' cost model may be off. If for some reason this causes issues later, the change should be guarded with 'hasProfile' check.
Fri, Jun 7
Thu, Jun 6
Wed, Jun 5
It is just more convenient for you to contribute to LLVM in the future. I can help you commit this time.
I suggest you apply for commit right first. There are likely fallouts that need to be dealt with.
lgtm. Watch out for problems on other platforms.
Tue, Jun 4
Mon, Jun 3
lgtm assuming the state tracking code gets removed (or put under NDEBUG in some way) after the cleanup.
Fri, May 31
For all the test cases update, please also validate if they make sense or not if possible.
once the cleanup is done, the state tracking should be removed as well.
Thu, May 30
I suggest the following path:
- add an option to enforce internal error on inconsistency; by default the inconsistency is ignored.
- fix the exposed root cause one by one
- turn on the option by default
Sounds reasonable -- the intention is clear with this -- the interface tells runtime to do whatever needed to ensure merging. Please send a patch with merging test case.
What I meant is that we should never see inconsistent state and instead of setting invalid state, we assert there. Ideally, the profile setting/update APIs should help user avoid creating the inconsistent state in the first place (i.e., catch the problem on the spot).
Wed, May 29
I think it is cleaner for user to make the intention clear by passing the flag if the file handle is passed to the runtime. Otherwise the runtime will have to guess/assume user's intention (e.g, also reopen the file with "r+b" mode for merging).
To support merging, the new interface needs to take more
Tue, May 28
Why is symbol remapping an issue? old MD5 --> input symbol name -> output symbol name --> mapped MD5sum ?
what is the main blocker for the longer term solution?
Longer term, the plan is to put a white list of the symbols into the profile data so that the compiler can decide if a function is newly created or simply cold.
There should be a test case.
Mon, May 27
May 24 2019
Does this patch handle the case in D62079? If yes, you can move the tests added in that patch here.
May 23 2019
May 22 2019
Indeed. The original one is O(N^2). LGTM.
was there a quadratic behavior before ? It seems linear before and after the patch -- but just cut the computation by a factor of 2 . Do you need an option to control the max chain length?
May 21 2019
Loop rotation not only affects fall through placement of incoming and exit edges, but internal fall through as well -- it converts the original backedge into a new fallthrough while converts an original internal fallthrough into the backedge. In other words, the overall cost/benefit needs to consider all changes. Doing pattern matching like this can lead to wrong decisions.
May 20 2019
I like the direction this patch is going, but I think it can do better.
May 15 2019
May 14 2019
May 10 2019
it is probably better to put the tests in the same file with different check prefixes.
May 9 2019
May 7 2019
May 6 2019
May 1 2019
Apr 30 2019
Apr 29 2019
Apr 24 2019
Apr 23 2019
Update related interfaces in ProfileSummaryInfo.
It is tempting to use enum in this case, however I don't see possible extension to the type (in forseable future), so perhaps keep this way.
Updated the patch to include more APIs.
Apr 19 2019
This is not completely NFC as it modifies branch weight update behavior (though correctly), so please do more pgo testings to avoid surprises (e.g, some weird dependencies on old behavior).
Looked at the Instruction::updateProfWeight() --- the part that update branch_weights seems bogus -- there is need need to scale branch weight at all.
Can you clarify? IIRC, we never update the per-call site profile meta data with inline transformations.
It is possible to for an entry with zero count -- as for instance in instrumentation PGO. Should it be fixed in place where the div by zero happens?
Apr 11 2019
Apr 10 2019
Mar 29 2019
Mar 28 2019
Mar 21 2019
Look fine in general. Easwaran can help with review of pass pipeline changes
Mar 19 2019
Missing unit tests for the feature.
Mar 12 2019
Mar 11 2019
Mar 7 2019
Mar 6 2019
Mar 5 2019
Mar 4 2019