Skip to content
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/5l: Suboptimal indirect function calls #4718

Closed
gopherbot opened this issue Jan 28, 2013 · 8 comments
Closed

cmd/5l: Suboptimal indirect function calls #4718

gopherbot opened this issue Jan 28, 2013 · 8 comments

Comments

@gopherbot
Copy link
Contributor

by Matthew.Horsnell:

What steps will reproduce the problem?
1. Compile a sequence of code such as http://play.golang.org/p/3JRV4kjthP on ARM using
the 5* cmds.
2. observe the code using objdump -d 

What is the expected output?

Best practice suggests that subroutine returns should use either pop (with pc in the
register list) or bx lr such that the return stack on modern hardware is utilised.

For reference the issue was fixed in gcc
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40887.

What do you see instead?
- No emitted "bx lr" sequences
- a mixture of pop {pc} (which is fine) and ldr pc, [sp], #offset (which is sub-optimal).

Which compiler are you using (5g, 6g, 8g, gccgo)?
5g

Which operating system are you using?


Which version are you using?  (run 'go version')
go version devel +f50a112bfe3b

Please provide any additional information below.
@remyoudompheng
Copy link
Contributor

Comment 1:

Labels changed: added performance, arch-arm.

@minux
Copy link
Member

minux commented Jan 29, 2013

Comment 2:

from my reading of cortex-a8 trm
(http://infocenter.arm.com/help/topic/com.arm.doc.ddi0344k/ch05s02s01.html):
Quote:
The following instructions cause a return stack pop if predicted:
BX r14
MOV pc, r14
LDM r13, {…pc}
LDR pc, [r13]
LDM r9, {..pc} (ThumbEE state only)
LDR pc, [r9] (ThumbEE state only).
The LDR instruction can use any of the addressing modes, as long as r13 is the base
register.
I think our use of "ldr pc, [sp], #offset" is included as it's using LDR pc, [r13} with
post increment
address mode.

Status changed to WaitingForReply.

@gopherbot
Copy link
Contributor Author

Comment 3 by Matthew.Horsnell:

I agree with your comment re ldr pc, {r13} my bad for listing that example.
I think the examples I had meant to include were:
ldr sp, [r1]
ldr pc, [r1, #4]
and 
add pc, lr
mov pc, lr
that doesn't obey the recommendations in the cortex-a8 trm, but perhaps not quite as
frequent as the the previous example suggested.

@gopherbot
Copy link
Contributor Author

Comment 4 by Matthew.Horsnell:

I agree with your comment re ldr pc, {r13} my bad for listing that example.
I think the examples I had meant to include were:
ldr sp, [r1]
ldr pc, [r1, #4]
and 
add pc, lr
perhaps replacing with the former with
ldr sp, [r1]
ldr pc, [sp, #4]
and the latter by
mov pc, lr
however these are not quite as frequently emitted as previously implied.

@rsc
Copy link
Contributor

rsc commented Jan 30, 2013

Comment 5:

Labels changed: added priority-later, go1.1maybe, removed priority-triage, go1.1.

Status changed to Thinking.

@robpike
Copy link
Contributor

robpike commented Mar 7, 2013

Comment 6:

Labels changed: removed go1.1maybe.

@minux
Copy link
Member

minux commented Jun 10, 2013

Comment 7:

This issue was updated by revision 35e1dea.

This CL makes BL (Rx) to use BLX Rx instead of:
MOV LR, PC
MOV PC, Rx
R=cshapiro, rsc
CC=dave, gobot, golang-dev
https://golang.org/cl/9669045

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 8:

I think everything is done now.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants