We certainly would like a way to precisely identify basic blocks in profiles so we can use that information for basic block level optimizations. That is what #59612 is intended to cover.

That issue doesn't cover precisely how the discriminator values are determined or how they get matched back to IR nodes and/or SSA blocks/values during the next build. It seems like this is something your design above tries to define, which is great.

That said, I don't quite follow exactly how you are defining them, or how propagation works as IR is mutated and eventually becomes SSA. Are you assigning discriminator values to high-level IR nodes (like ir.ForStmt) and then propagating through mutations and SSA?

I suspect this kind of propagation will get very complicated. I'd be tempted, at least for an initial version, to keep everything in SSA. Discriminators are based on basic block numbers, and optimizations prior to SSA simply cannot use them. Even in this case, I think tracking correlation to the profile through the different layers of SSA passes will be difficult.

I happen to be working on a prototype to assign discriminators to each PC, and plumb that into the binary metadata and the pprof profile. Once I have that done, you may be able to use this prototype to play with the PGO side of matching the samples-with-discriminators back to the build and applying basic block optimizations.

cc @cherrymui @aclements @jinlin-bayarea

@prattmic thank you for your answer.

Yes, you are right that discriminators are based on the basic blocks. My proposal needs the information about the column for the sampled instruction, so maybe the dwarf column information will be more suitable for my proposal. Thanks for highlighting that. Answering the question about the discriminator assignment - I do not do that for now (and need not discriminators itself, but the column number).

Yet, the column information is useful, we can load the counters to the AST nodes without it. The profile may be less precise, but still we can implement the profile-based optimizations. My motivation for loading the counters to the AST level is an opportunity to implement not only inline optimization on the AST level, so I think we should start loading the profile counters at the AST level.

Change https://go.dev/cl/560781 mentions this issue: PROTOTYPE: cmd/compile,runtime: add discriminator, plumb to pprof

https://go.dev/cl/560781 plumbs a discriminator value from the compiler to the pprof Line.Column field (even though the value is not actually the column number.

Feel free to use this if you'd like to play with using discriminators for PGO. If you'd like just the column number itself, see my comment at https://go-review.googlesource.com/c/go/+/560781/2/src/cmd/internal/obj/pcln.go.

I don't see why this has to be a proposal; taking it out of the proposal process.

Change https://go.dev/cl/564055 mentions this issue: [WIP] DO NOT REVIEW cmd/compile: add basic block counters

I implemented some proof-of-concept prototype for basic block counters. Currently done:

• Loading counters to AST nodes by line number information
• The AST counter propagation algorithm
• Moving counters to SSA nodes from AST nodes
• Adding profile usage in basic block layout pass (just proof-of-concept)
• Print counters to the html dumps

The patch shows how we can load basic block counters from the pprof file, how we can propagate them, and how to use them. The patch was tested on the go1 benchmark set, and it showed that depending on bb-layout pass the test fannkuch can be faster from 3.5% to 12.2% on the intel core machine (the xeon machine does not show this improvement).

To use basic block counters you should add an option -bbpgo: go test -a -bbpgo -pgo=go1.prof

This patch just shows the main idea of basic block counters loading. Before we can use it in the compiler, the following steps should be done:

• Improve the counter propagation on the AST. Currently it does not take in account the returns from the middle of function. Also switches are not evaluated as needed.
• Implement the counter corrections after inline
• Improve the counter moving to SSA. At least we should process PanicBounds generation in a proper way.
• Improve basic block layout and register allocation passes with profile information
• Fix formatting when generating html dumps

This patch itself is not for review, but I would like to get some feedback for the general approach - the propagation algorithm and the Node structure modifications itself. Please, take a look at the irgraph.go and the ssa part.

cc @cherrymui @aclements @jinlin-bayarea @prattmic

Status update:

• Improved profile loading to AST nodes. Currently the propagation algorithm takes in account possible returns in the middle of the function; improved the counter correction for branches; some other fixes to make counter loading and propagation more precise.
• Improved profile correction for AST and SSA nodes transformations.
• Fixed html dumps formatting. Now it looks pretty good.
• Added IR profile check, but currently it is not usable in general case

During work with this feature, I came across some problems:

• Edge counters. I made an attempt to add counters to the edges, as it should simplify pgo-based algorithms, but adding them breaks the compiler. Even if we do not have any reads of edges counters (just add a field and set any value), we have a compiler break. I think there is a place, where two edge objects are compared by value, and different counters in the edges influence the result of such comparison. To my pity, I did not find this place
• ONAME and OLITERAL nodes. For one name and one literal we generate only one node and use its copies. That leads to the situation of an incorrect profile. For example:

if cond {
 // Hot path
 use A // ONAME node with big counter
} else {
 // Cold path
 use A // same ONAME node with the same big counter. But here it should be small
}

Currently I set zero counters to the ONAME and OLITERAL nodes, but hope a better solution can be found

• IR dumps subsystem. Currently html dumps are written at the ssagen stage, but between AST creation and SSA generation, AST is transformed a few times. It would be useful to dump the AST IR after passes as well, but it seems, that it is too complicated now.

Further plans:

• Continue improving propagation algorithm and counter correction for best precision
• Make code refactoring
• Add tests for pgo
• Improve optimization passes to use basic block profiling

Status update:

• Improved propagation algorithm. Added in places, where AST nodes are created during its transformations; now before ssagen the propagation is launched the second time (as a lot of new nodes created); other algorithm changes
• Huge code refactoring and cleanup
• Fixes for dumps of nodes with counters (ащк html and dumps alike)
• Added tests for counters
• Added choosing of likely branch based on counter value

For now, there are lots of things to do, but the basic block counters subsystem already allows to be used by pass developers. I ask for the review of this patch. After that I will be able to continue improving the basic block counters subsystem.

Alternative approach by @jinlin-bayarea: https://go.dev/cl/571535

I would like to discuss the current status of basic-block counters implementation. According to the issue #62463, we have a list of wanted PGO optimizations. Some of them are related to the AST level, and some of them to the SSA level. Now we have two prototypes for basic block counters implementation:

1. Loading counters on the AST (https://go.dev/cl/564055)
2. Loading counters on the SSA (https://go.dev/cl/571535)

Both approaches have advantages and disadvantages, and I would like to understand if I should continue improving the first one.

The advantages of loading counters on AST:

• It allows the profile guided optimizations on AST level
• The counter propagation is more simple (as the tree as acyclic)
• We can use counters on the SSA phase from the beginning

The problems of loading counters on AST:

• We need corrections during optimizations (this can be simplified in current implementation)
• The converting counters to basic blocks on the ssagen pass may be tricky sometimes
• May be we need to add corrections to other passes, that change cfg

The advantages of loading counters on SSA:

• More precise counters on basic blocks (as they are close to the blocks in the executable)
• More simple integration as lesser number of passes should be modified and lesser corrections should be done

The problems of loading counters on SSA:

• The counters should be loaded in the end of optimization pipeline, so lesser ideas from "PGO umbrella" may be implemented

In general, I believe that the loading counters to the AST is useful, as it gives us many opportunities for profile optimizations in all the parts of the compiler. I think that we even can use both approaches at the same time, as they do not conflict: for early optimizations we can use less precise counters and for late optimizations - more precise.

Please, share your thoughts on this idea - it is important to understand the community point of view.

CC @jinlin-bayarea @cherrymui @aclements @jinlin-bayarea @prattmic