code review 10785043: cmd/6g, cmd/8g: prevent constant propagation of non-con...

cmd/6g, cmd/8g: prevent constant propagation of non-constant LEA.

Fixes issue 5809.

[DMorsing, 2013-06-29 13:17:17]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[dave_cheney.net, 2013-06-29 13:18:58]

Thanks for fixing this so quickly. Is {6,8}c affected ?

On Sat, Jun 29, 2013 at 11:17 PM,  <daniel.morsing@gmail.com> wrote:
> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> cmd/6g, cmd/8g: prevent constant propagation of non-constant LEA.
>
> Fixes issue 5809.
>
> Please review this at https://codereview.appspot.com/10785043/
>
> Affected files:
>   M src/cmd/6g/peep.c
>   M src/cmd/8g/peep.c
>   A test/fixedbugs/issue5809.go
>
>
> Index: src/cmd/6g/peep.c
> ===================================================================
> --- a/src/cmd/6g/peep.c
> +++ b/src/cmd/6g/peep.c
> @@ -154,6 +154,7 @@
>                 case ALEAQ:
>                         if(regtyp(&p->to))
>                         if(p->from.sym != S)
> +                       if(p->from.index == D_NONE)
>                                 conprop(r);
>                         break;
>
> Index: src/cmd/8g/peep.c
> ===================================================================
> --- a/src/cmd/8g/peep.c
> +++ b/src/cmd/8g/peep.c
> @@ -147,6 +147,7 @@
>                 case ALEAL:
>                         if(regtyp(&p->to))
>                         if(p->from.sym != S)
> +                       if(p->from.index == D_NONE)
>                                 conprop(r);
>                         break;
>
> Index: test/fixedbugs/issue5809.go
> ===================================================================
> new file mode 100644
> --- /dev/null
> +++ b/test/fixedbugs/issue5809.go
> @@ -0,0 +1,27 @@
> +// run
> +
> +// Copyright 2013 The Go Authors.  All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +// issue 5609: 6g and 8g attempted to constant propagate indexed LEA
> +
> +package main
> +
> +import "fmt"
> +
> +func main() {
> +       const d16 = "0123456789ABCDEF"
> +       k := 0x1234
> +       var x [4]byte
> +
> +       x[0] = d16[k>>12&0xf]
> +       x[1] = d16[k>>8&0xf]
> +       x[2] = d16[k>>4&0xf]
> +       x[3] = d16[k&0xf]
> +
> +       if x != [4]byte{'1','2','3','4'} {
> +               fmt.Println(x)
> +               panic("x != [4]byte{'1','2','3','4'}")
> +       }
> +}
>
>
> --
>
> ---You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[DMorsing, 2013-06-29 13:26:13]

On 2013/06/29 13:18:58, dfc wrote:
> Thanks for fixing this so quickly. Is {6,8}c affected ?
> 

No, they don't constant propagate. Neither does 5g.

[rsc, 2013-07-01 21:41:09]

https://codereview.appspot.com/10785043/diff/6001/src/cmd/6g/peep.c
File src/cmd/6g/peep.c (right):

https://codereview.appspot.com/10785043/diff/6001/src/cmd/6g/peep.c#newcode157
src/cmd/6g/peep.c:157: if(p->from.index == D_NONE)
|| p->from.index == D_CONST?

https://codereview.appspot.com/10785043/diff/6001/src/cmd/8g/peep.c
File src/cmd/8g/peep.c (right):

https://codereview.appspot.com/10785043/diff/6001/src/cmd/8g/peep.c#newcode150
src/cmd/8g/peep.c:150: if(p->from.index == D_NONE)
same

[DMorsing, 2013-07-04 15:06:52]

Hello golang-dev@googlegroups.com, dave@cheney.net, rsc@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[nigeltao, 2013-07-05 01:29:44]

LGTM.

https://codereview.appspot.com/10785043/diff/14001/test/fixedbugs/issue5809.go
File test/fixedbugs/issue5809.go (right):

https://codereview.appspot.com/10785043/diff/14001/test/fixedbugs/issue5809.g...
test/fixedbugs/issue5809.go:7: // issue 5609: 6g and 8g attempted to constant
propagate indexed LEA
s/5609/5809/

[DMorsing, 2013-07-05 14:11:32]

*** Submitted as https://code.google.com/p/go/source/detail?r=cf792c00f410 ***

cmd/6g, cmd/8g: prevent constant propagation of non-constant LEA.

Fixes issue 5809.

R=golang-dev, dave, rsc, nigeltao
CC=golang-dev
https://codereview.appspot.com/10785043