The class wraps a uint64_t and an enum to represent the type of profile
count (real and synthetic) with some helper methods.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
include/llvm/IR/Function.h | ||
---|---|---|
237 ↗ | (On Diff #129165) | Perhaps also split sample count and instrumentation count? |
246 ↗ | (On Diff #129165) | Why is there a need for the static member? |
254 ↗ | (On Diff #129165) | Do we need to interfaces for the same thing? I suggest removing one. |
255 ↗ | (On Diff #129165) | Is this interface useful? |
260 ↗ | (On Diff #129165) | Perhaps also add a parameter for the type? This can avoid dropping count silently when the type mismatches. |
include/llvm/IR/Function.h | ||
---|---|---|
237 ↗ | (On Diff #129165) | Right now the name used in metadata is used to differentiate real and synthetic counts. We currently use function_entry_count which I treat as real and then add a new synthetic_entry_count which is treated as of type synthetic. We use function_entry_count for both sample and instrumented PGO. Do you want to use separate names there? |
246 ↗ | (On Diff #129165) | I create an unqiue ProfileCount with type set to Invalid and use that when getInvalid() is called. I could construct an invalid object everytime getInvalid is called but I thought this is preferable. |
260 ↗ | (On Diff #129165) | Not sure what you mean here. We have several places where we get the count and clone it to a different function or scale the value. All these places, we don't need to know the type of the count is, but want to preserve the same type after manipulating the value. This can be done by doing something like Count = F.getEntryCount(); So there is no issue of type mismatch here. |
include/llvm/IR/Function.h | ||
---|---|---|
237 ↗ | (On Diff #129165) | We can split the enum, but keep the interfaces as they are. We may find the need to differentiate later. |
246 ↗ | (On Diff #129165) | Why is using static member more preferrable? Or make it a const object? |
260 ↗ | (On Diff #129165) | The case I am thinking is the mixed profile case: e.g, the function has real profile count but overriden by this method with synthetic count. Maybe change the name to 'updateCount' or resetCount ? |
Also expanded getEntryCount to read synthetic_function_entry_count.
include/llvm/IR/Function.h | ||
---|---|---|
237 ↗ | (On Diff #129165) | As discussed offline, there is not enough information in the metdata to differentiate the two. We can go down that route later if needed. |
246 ↗ | (On Diff #129165) | Removed the static |
255 ↗ | (On Diff #129165) | Useful in unit tests and I have also added a assert in setEntryCount where this is used. |
260 ↗ | (On Diff #129165) | The right place for that is setEntryCount. This one just changes the value of a ProfileCount object. Added an assert in setEntryCount to check either existing type is invalid or the same as what you're setting. |