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

Issue 6620064: code review 6620064: cmd/5g: avoid temporary in slice bounds check (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by dfc
Modified:
1 year, 6 months ago
Reviewers:
CC:
rsc, remyoudompheng, minux, golang-dev
Visibility:
Public.

Description

cmd/5g: avoid temporary in slice bounds check

before

func addr(s[]int) *int {
        return &s[2]
   10c1c:       e28d0008        add     r0, sp, #8
   10c20:       e5901004        ldr     r1, [r0, #4]
   10c24:       e3a02002        mov     r2, #2
   10c28:       e1510002        cmp     r1, r2
   10c2c:       8a000000        bhi     10c34 <main.addr+0x34>
   10c30:       eb0035e6        bl      1e3d0 <runtime.panicindex>
   10c34:       e5900000        ldr     r0, [r0]
   10c38:       e2800008        add     r0, r0, #8
   10c3c:       e58d0014        str     r0, [sp, #20]
   10c40:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

after

func addr(s[]int) *int {
	return &s[2]
   10c1c:       e28d0008        add     r0, sp, #8
   10c20:       e5901004        ldr     r1, [r0, #4]
   10c24:       e3510002        cmp     r1, #2
   10c28:       8a000000        bhi     10c30 <main.addr+0x30>
   10c2c:       eb0035e6        bl      1e3cc <runtime.panicindex>
   10c30:       e5900000        ldr     r0, [r0]
   10c34:       e2800008        add     r0, r0, #8
   10c38:       e58d0014        str     r0, [sp, #20]
   10c3c:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

Also, relax gcmp restriction that 2nd operand must be a register. A followup
CL will address the remaining TODO items.

Patch Set 1 #

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

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

Patch Set 4 : diff -r 187194a52289 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -6 lines) Patch
M src/cmd/5g/cgen.c View 1 2 chunks +2 lines, -5 lines 0 comments Download
M src/cmd/5g/gsubr.c View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5
dfc
Hello rsc@golang.org, remyoudompheng@gmail.com, minux.ma@gmail.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
1 year, 6 months ago #1
minux
what are the remaining TODO items?
1 year, 6 months ago #2
rsc
LGTM
1 year, 6 months ago #3
dfc
See the note by kabi in gsubr.c#gcmp On 7 Oct 2012 04:26, <minux.ma@gmail.com> wrote: > ...
1 year, 6 months ago #4
dfc
1 year, 6 months ago #5
*** Submitted as http://code.google.com/p/go/source/detail?r=42ac75ca0e72 ***

cmd/5g: avoid temporary in slice bounds check

before

func addr(s[]int) *int {
        return &s[2]
   10c1c:       e28d0008        add     r0, sp, #8
   10c20:       e5901004        ldr     r1, [r0, #4]
   10c24:       e3a02002        mov     r2, #2
   10c28:       e1510002        cmp     r1, r2
   10c2c:       8a000000        bhi     10c34 <main.addr+0x34>
   10c30:       eb0035e6        bl      1e3d0 <runtime.panicindex>
   10c34:       e5900000        ldr     r0, [r0]
   10c38:       e2800008        add     r0, r0, #8
   10c3c:       e58d0014        str     r0, [sp, #20]
   10c40:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

after

func addr(s[]int) *int {
	return &s[2]
   10c1c:       e28d0008        add     r0, sp, #8
   10c20:       e5901004        ldr     r1, [r0, #4]
   10c24:       e3510002        cmp     r1, #2
   10c28:       8a000000        bhi     10c30 <main.addr+0x30>
   10c2c:       eb0035e6        bl      1e3cc <runtime.panicindex>
   10c30:       e5900000        ldr     r0, [r0]
   10c34:       e2800008        add     r0, r0, #8
   10c38:       e58d0014        str     r0, [sp, #20]
   10c3c:       e49df004        pop     {pc}            ; (ldr pc, [sp], #4)

Also, relax gcmp restriction that 2nd operand must be a register. A followup
CL will address the remaining TODO items.

R=rsc, remyoudompheng, minux.ma
CC=golang-dev
http://codereview.appspot.com/6620064
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5