New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile, cmd/internal/obj/ppc64: generate the likely/unlikely bit on branches for ppc64x #17235

Open
laboger opened this Issue Sep 26, 2016 · 4 comments

Comments

Projects
None yet
6 participants
@laboger
Contributor

laboger commented Sep 26, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version devel +375092b Mon Sep 26 15:46:44 2016 +0000 linux/ppc64le

What operating system and processor architecture are you using (go env)?

Ubuntu 16.04

What did you do?

Perused generated ppc64x code.

What did you expect to see?

Branch likely flag set for many commonly generated if-then-else sequences when the 'then' block should rarely be taken.

What did you see instead?

I don't see the likely/unlikely flag set it by golang generated code except in asm files.

The asm9.go file has support to set the likely/unlikely flag but the branch asm opcodes don't currently have an operand combination which indicates when it should be done.

I plan to add support for an operand with the branch that indicates when to set likely/unlikely, then update some of the sequences that should have the flag set. This includes morestack_noctxt in the prolog, and the runtime checks that result in panics once I figure out where those are.

@randall77

This comment has been minimized.

Contributor

randall77 commented Sep 26, 2016

cmd/compile/internal/ssa/block.go:Block has a branch prediction field (called Likely).
You can use that in cmd/compile/internal/ppc64/ssa.go to populate a branch Prog with the right data.
For amd64 we use Prog.From=={Const,0} for unlikely taken and {Const,1} for likely taken. Use that encoding for PPC as well if it makes sense.
See cmd/compile/internal/amd64/ssa.go for an example of how to do that.

@quentinmit quentinmit changed the title from cmd/compile, cmd/internal/obj/ppc64 generate the likely/unlikely bit on branches for ppc64x to cmd/compile, cmd/internal/obj/ppc64: generate the likely/unlikely bit on branches for ppc64x Oct 3, 2016

@quentinmit quentinmit added this to the Go1.8 milestone Oct 3, 2016

@quentinmit quentinmit added the NeedsFix label Oct 3, 2016

@laboger

This comment has been minimized.

Contributor

laboger commented Oct 14, 2016

I will not be submitting a change for this in Go 1.8. The performance testing I've done for this had mixed results and will require more investigation to determine if it is worth it.

@rsc rsc modified the milestones: Go1.9, Go1.8 Oct 21, 2016

@laboger

This comment has been minimized.

Contributor

laboger commented Jan 18, 2017

I've done some investigation on this, setting the likely bit based on the end of block information. However that does not always provide an improvement, it varies quite a bit and in many cases gets worse. I have not investigated further to try and determine if the information provided is not dependable for ppc64x or if there is some other reason.

If I were to set this at all, I'd prefer to only set it in the obvious cases, such as when there is an if check for a panic condition and then the panic path marked as unlikely. Not sure what the best way is to determine that.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Jan 18, 2017

Thanks for the research. I've done the analysis in the past (other languages, other compilers) to do the "this will certainly fail, so it's certainly not a critical path" likeliness check, and I think we could do that here, too.

I have an item on my to-do list to write up a list of little useful projects, and that's one of them.
(Tentative list: improve spill location in register allocation; improve escape analysis w/ interprocedural use info for interface-typed call args; spec out loop unrolling within SSA form for both optimization and the reschedule check; this finer-grained "likely" information. I think Keith has a similar list).

@randall77 randall77 modified the milestones: Go1.10, Go1.9 May 31, 2017

@bradfitz bradfitz modified the milestones: Go1.10, Go1.11 Nov 28, 2017

@bradfitz bradfitz modified the milestones: Go1.11, Unplanned May 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment