Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: Opportunity for de-pessimization in provably const range loops #8346

Open
thockin opened this issue Jul 9, 2014 · 6 comments

Comments

@thockin
Copy link

commented Jul 9, 2014

What does 'go version' print?

go version go1.3 linux/amd64

What steps reproduce the problem?

Using the 2-return 'range' over a slice of structs will copy each struct, even if the
struct is provably not modified in the loop.  This is a premature pessimization that the
compiler could and should eliminate.  Ideally the spec would guarantee this optimization
(a la RVO in C++) so people could count on it and maybe even warn when it is not true.

The for-each construct is much nicer to read and write, but this behavior sort of makes
it a non-starter for large data.  This is discussed in many places on the web, with the
general recommendation of "just iterate on index".  This is unfortunate for
the language.

Alternatively: some syntax to say explicitly "iterate by reference" or by
pointer.

Ian asked me to file this issue.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2014

Comment 1:

I don't think any spec or syntax change is needed.  As you say, you can already
guarantee the behaviour you want with more awkward syntax, so I don't think we need to
add another kind of syntax for it.  In general, though, the compiler should generate
better code for this.

Labels changed: added release-none, repo-main, suggested.

@dvyukov

This comment has been minimized.

Copy link
Member

commented Jul 9, 2014

Comment 2:

Note that for the following loop:
for _, v := range s {
  ...
  foo()
  ...
  use(v)
}
you can't generally prove (w/o global analysis) that it does not modify elements of the
slice. Because foo can be:
func foo() {
  c <- true
}
And the other side of the chan can be:
for i := range s {
  <-c
  s[i] = 0
}

@bradfitz bradfitz removed the new label Dec 18, 2014

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@rsc rsc changed the title cmd/gc: Opportunity for de-pessimization in provably const range loops cmd/compile: Opportunity for de-pessimization in provably const range loops Jun 8, 2015

@agnivade

This comment has been minimized.

Copy link
Member

commented Jun 16, 2018

/cc @rasky for possibility of doing this in the prove pass.

@zdjones

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2019

Could you provide a short example? I can look into whether this would fit into the prove pass.

I don't know much about it, but maybe the copyelim pass would be a possibility?

Would this currenlty be possible earlier on in walkrange (

func walkrange(n *Node) *Node {
). I'm not sure who that would be.

@rasky

This comment has been minimized.

Copy link
Member

commented Jul 5, 2019

I don't think this fits prove.

@zdjones

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2019

I don't think this fits prove.

Agreed. The more I read about RVO and copy elision, the more I think it's not for prove.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.