Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2997)

Issue 158180047: code review 158180047: runtime/cgo: encode BLX directly, fixes one clang build error on arm (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 6 months ago by dave
Modified:
9 years, 6 months ago
Reviewers:
gobot, iant
CC:
iant, minux, golang-codereviews
Visibility:
Public.

Description

runtime/cgo: encode BLX directly, fixes one clang build error on arm Fixes issue 8348. Trying to work around clang's dodgy support for .arch by reverting to the external assembler didn't work out so well. Minux had a much better solution to encode the instructions we need as .word directives which avoids .arch altogether. I've confirmed with gdb that this form produces the expected machine code Dump of assembler code for function crosscall_arm1: 0x00000000 <+0>: push {r4, r5, r6, r7, r8, r9, r10, r11, r12, lr} 0x00000004 <+4>: mov r4, r0 0x00000008 <+8>: mov r5, r1 0x0000000c <+12>: mov r0, r2 0x00000010 <+16>: blx r5 0x00000014 <+20>: blx r4 0x00000018 <+24>: pop {r4, r5, r6, r7, r8, r9, r10, r11, r12, pc} There is another compilation failure that blocks building Go with clang on arm # ../misc/cgo/test # _/home/dfc/go/misc/cgo/test /tmp/--407b12.s: Assembler messages: /tmp/--407b12.s:59: Error: selected processor does not support ARM mode `blx r0' clang: error: assembler command failed with exit code 1 (use -v to see invocation) FAIL _/home/dfc/go/misc/cgo/test [build failed] I'll open a new issue for that

Patch Set 1 #

Patch Set 2 : diff -r 663d8fa280489a14dc5b1c2afece6c3d9babc662 https://code.google.com/p/go #

Patch Set 3 : diff -r 663d8fa280489a14dc5b1c2afece6c3d9babc662 https://code.google.com/p/go #

Patch Set 4 : diff -r 663d8fa280489a14dc5b1c2afece6c3d9babc662 https://code.google.com/p/go #

Patch Set 5 : diff -r 4b5ffe0e679e6ad9ad4791d4947092905be10fa7 https://code.google.com/p/go #

Patch Set 6 : diff -r 4b5ffe0e679e6ad9ad4791d4947092905be10fa7 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -9 lines) Patch
M src/runtime/cgo/gcc_arm.S View 1 2 2 chunks +6 lines, -9 lines 0 comments Download

Messages

Total messages: 5
dave_cheney.net
Hello iant@golang.org, minux@golang.org (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
9 years, 6 months ago (2014-10-22 00:10:01 UTC) #1
minux
Because I suggested this approach, I will leave this CL to iant (or others) to ...
9 years, 6 months ago (2014-10-22 00:18:19 UTC) #2
iant
LGTM
9 years, 6 months ago (2014-10-22 01:13:17 UTC) #3
dave_cheney.net
*** Submitted as https://code.google.com/p/go/source/detail?r=66a91d217fcb *** runtime/cgo: encode BLX directly, fixes one clang build error on ...
9 years, 6 months ago (2014-10-22 01:21:20 UTC) #4
gobot
9 years, 6 months ago (2014-10-22 13:46:42 UTC) #5
Message was sent while issue was closed.
This CL appears to have broken the plan9-amd64-aram builder.
See http://build.golang.org/log/552cc4fea39e2a719fe1fc099a7dae4db12cb6a7
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b