gmane.comp.lang.go.devel

Re: code review 6258050: cmd/cgo: give error if imported "C" but not used (issue 6258050)

2012-05-26T04:30:22

as explained in
https://groups.google.com/group/golang-nuts/browse_thread/thread/b7276c91b9cd4df8#
I think unused import "C" might not match the user's expectation
(only uses the side effect of that C code).

if we must preserve the validity of all Go 1 programs, how about
change it to a warning? (I still believe this is a cgo bug, if it's,
then we are free to correct it)

I've found only one use of unused import "C", that is, packageA
links in a C library, but doesn't use it; while another packageB
imports packageA, use the linked C library without specifying
LDFLAGS again; that will work, but it's very weird and arguably
not idiomatic Go.

http://codereview.appspot.com/6258050/

Re: code review 6258050: cmd/cgo: give error if imported "C" but not used (issue 6258050)

Rémy Oudompheng — 2012-05-25T19:45:59

I thought it was incompatible with the 'Go 1' contract to make
programs that previously compile not compile anymore?

code review 6258050: cmd/cgo: give error if imported "C" but not used (issue 6258050)

2012-05-25T17:55:04

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org (cc: golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

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


Description:
cmd/cgo: give error if imported "C" but not used
     runtime/cgo is a special case, so it uses a workaround.

Please review this at http://codereview.appspot.com/6258050/

Affected files:
   M src/cmd/cgo/ast.go
   M src/pkg/runtime/cgo/cgo.go


Index: src/cmd/cgo/ast.go
===================================================================
--- a/src/cmd/cgo/ast.go
+++ b/src/cmd/cgo/ast.go
< at >< at > -58,6 +58,7 < at >< at >

  // In ast1, find the import "C" line and get any extra C preamble.
  sawC := false
+var importCPos token.Pos
  for _, decl := range ast1.Decls {
  d, ok := decl.(*ast.GenDecl)
  if !ok {
< at >< at > -69,6 +70,7 < at >< at >
  continue
  }
  sawC = true
+importCPos = s.Path.Pos()
  if s.Name != nil {
  error_(s.Path.Pos(), `cannot rename import "C"`)

Re: code review 6248049: cmd/6g, cmd/8g: move panicindex calls out of line (issue 6248049)

Robert Griesemer — 2012-05-25T17:37:10

This is great.

I've done this in the code gens of V8 and the JVM in the past - always
moved every code branch that was unlikely to be executed the end. The
IR had a special mechanism just for that (and often those stubs - of
which there were many - could be shared as well, thus reducing the
overall code size).

Results look good. Don't know what happened to Mandelbrot. It doesn't
use complex numbers (the one in shootout).

- gri



On Thu, May 24, 2012 at 11:44 PM,  <rsc-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org> wrote:

Re: code review 6225051: 5a, 5l, math: Add support for ABSD, ABSF floating point... (issue 6225051)

Russ Cox — 2012-05-25T16:14:01

Don't you also have to add case 0xeeb10bc4 to src/pkg/runtime/softfloat_arm.c?

Russ

Re: Re: wanna atomic func..

Russ Cox — 2012-05-25T16:05:27

The general discussion mailing list golang-nuts-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org would
be a more appropriate place for this thread.  Please try asking there.

Russ

Re: wanna atomic func..

まにらふ — 2012-05-25T15:40:10

fmt.printf(%d)    ///////  mistake
fmt.Printf("%d",i)  /////// correct

On 5月25日, 午後9:45, まにらふ <plu...-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: Showstoppers with cross-compilation to TI C6x architecture

Ian Lance Taylor — 2012-05-25T13:39:39


Without libffi support reflect.Call doesn't work and
runtime.Setfinalizer doesn't work.  Everything else should work.  If you
had to you could get runtime.Setfinalizer to  work, but reflect.Call
would be hard.

I took a quick look at the C6x calling convention, and it's quite
simple.  A libffi port should be fairly straightforward.



Segmented stacks are definitely desirable but many Go programs will run
without them.



Yes, getcontext/setcontext/makecontext are easy to implement.
getcontext/setcontext are basically the same as setjmp/longjmp.
makecontext lets you set the stack pointer and the PC.

Ian

code review 6225051: 5a, 5l, math: Add support for ABSD, ABSF floating point... (issue 6225051)

2012-05-25T13:28:21

R=rsc

(sent by gocodereview)

Re: code review 6252048: cmd/gc: fix parallel assignment in range (issue 6252048)

2012-05-25T13:21:54

lgtm



http://codereview.appspot.com/6252048/

Re: code review 6258049: cmd/8c: better fix for 64-bit register smash (issue 6258049)

2012-05-25T13:18:39

lgtm



http://codereview.appspot.com/6258049/

wanna atomic func..

まにらふ — 2012-05-25T12:42:55

4 exmple

a:=int{1,2,3,4,5}
b:=int{3,2,4}

for _,i:=range a|b {
  fmt.printf(%d)            // stdout is 1 2 3 4 5
}

for _,i:=range a&b{
  fmt.printf(%d)            // stdout is 2 3 4
}

if its logical array function, very useful..

wanna atomic func..

まにらふ — 2012-05-25T12:45:47

4 exmple

a:=[]int{1,2,3,4,5}
b:=[]int{3,2,4}

for _,i:=range a|b {
  fmt.printf(%d)            // stdout is 1 2 3 4 5
}

for _,i:=range a&b{
  fmt.printf(%d)            // stdout is 2 3 4
}

if its logical array function, very useful..

Showstoppers with cross-compilation to TI C6x architecture

Timon ter Braak — 2012-05-25T09:53:38

I have a cross-compilation toolchain (derived from codesourcery) towards 
the C6x architecture from TI ( a HOST to C6x-uclibc chain).
The toolchain is able to produce C,C++ compilers.

I started this effort to attempt to run Go on this multi-core DSP.
I know that C6x is a specialized architecture and freshly supported. But 
I would like to use Go on it for it's robustness and the convenience of 
Go operating in a heterogeneous platform.

Starting with GCC 4.7, both Go and C6x are supported. However, the 
combination of these two is not. I will outline my findings, and would 
like some feedback whether this combo is a nogo ;) or that I should 
invest some porting effort to do so.

Currently, Go builds upon two pillars that are currently prohibiting the 
build:

* libffi:
   the C6x architecture is not available within libffi
   the runtime can be build without libffi support, at the cost of 
broken reflection support; we could port libffi to the C6x architecture 
to fix this. So, for an initial attempt this i

Re: Re: code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

minux — 2012-05-25T07:04:51


I think we can use crypto++: individual files are placed in public domain.

code review 6248049: cmd/6g, cmd/8g: move panicindex calls out of line (issue 6248049)

2012-05-25T06:44:59

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
cmd/6g, cmd/8g: move panicindex calls out of line

The old code generated for a bounds check was
                 CMP
                 JLT ok
                 CALL panicindex
         ok:
                 ...

The new code is (once the linker finishes with it):
                 CMP
                 JGE panic
                 ...
         panic:
                 CALL panicindex

which moves the calls out of line, putting more useful
code in each cache line.  This matters especially in tight
loops, such as in Fannkuch.  The benefit is more modest
elsewhere, but real.

 From test/bench/go1, amd64:

benchmark                old ns/op    new ns/op    delta
BenchmarkBinaryTree17   6096092000   6088808000   -0.12%
BenchmarkFannkuch11     6151404000   4020463000  -34.64%
BenchmarkGobDecode        28990050     28894630   -0.33%

Re: code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

Russ Cox — 2012-05-25T06:21:57

Actually, screw it.

Re: code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

2012-05-25T06:22:05

*** Abandoned ***

http://codereview.appspot.com/6259043/

Re: code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

Russ Cox — 2012-05-25T06:21:08

PTAL

Re: code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

Russ Cox — 2012-05-25T06:21:03

Never mind, you're right: we shouldn't bother with OpenSSL.  Sigh.

Luckily the author of the assembly-generating Perl has made the SHA1
available under a new BSD license, so we can use that.  Now this CL is
just the SHA1 assembly and some refactoring to make assembly easier to
integrate in the other places, but no assembly yet.

Russ

Re: code review 6259043: crypto: amd64 assembly for hashes (issue 6259043)

Florian Weimer — 2012-05-25T06:19:52

Sorry, I didn't select enough lines in the tool.  This is equally
problematic:

* 3. All advertising materials mentioning features or use of this
+ * software must display the following acknowledgment:
+ * "This product includes software developed by the OpenSSL Project
+ * for use in the OpenSSL Toolkit. (http://www.openssl.org/)"

+ * 6. Redistributions of any form whatsoever must retain the following
+ * acknowledgment:
+ * "This product includes software developed by the OpenSSL Project
+ * for use in the OpenSSL Toolkit (http://www.openssl.org/)"

+ * 3. All advertising materials mentioning features or use of this software
+ * must display the following acknowledgement:
+ * "This product includes cryptographic software written by
+ * Eric Young (eay-DzJ7PIepUGbby3iVrkZq2A< at >public.gmane.org)"
+ * The word 'cryptographic' can be left out if the rouines from the library
+ * being used are not cryptographic related :-).

At the very least, you should include these required notices in the
top-level license f