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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 5 months ago by dfc
Modified:
11 years, 5 months ago
Reviewers:
CC:
rsc, remyoudompheng, minux1, 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
11 years, 5 months ago (2012-10-06 11:55:46 UTC) #1
minux1
what are the remaining TODO items?
11 years, 5 months ago (2012-10-06 17:26:07 UTC) #2
rsc
LGTM
11 years, 5 months ago (2012-10-06 21:54:55 UTC) #3
dfc
See the note by kabi in gsubr.c#gcmp On 7 Oct 2012 04:26, <minux.ma@gmail.com> wrote: > ...
11 years, 5 months ago (2012-10-06 22:19:56 UTC) #4
dfc
11 years, 5 months ago (2012-10-07 00:37:27 UTC) #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 f62528b