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
Open
Labels
Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@thockin
Copy link

thockin 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
Copy link
Contributor

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
Copy link
Member

dvyukov 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
}

@thockin thockin added new Suggested Issues that may be good for new contributors looking for work to do. labels Jul 9, 2014
@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc rsc added this to the Unplanned milestone 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
Copy link
Contributor

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

@zdjones
Copy link
Contributor

zdjones 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
Copy link
Member

rasky commented Jul 5, 2019

I don't think this fits prove.

@zdjones
Copy link
Contributor

zdjones 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
Labels
Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

8 participants