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

EncodeUtf8 iterator implementation results in bad code #35099

Closed
nagisa opened this issue Jul 28, 2016 · 4 comments
Closed

EncodeUtf8 iterator implementation results in bad code #35099

nagisa opened this issue Jul 28, 2016 · 4 comments

Comments

@nagisa
Copy link
Member

nagisa commented Jul 28, 2016

Provided #35098 (more precisely f729e17 commit) lands, iterating x.encode_utf8().as_slice() results in considerably better code compared to x.encode_utf8().

One major problem with the EncodeUtf8 iterator is the fact that it uses checked indexing, despite it being obvious this iterator cannot index out-of-bounds. However, even if that problem is fixed, noticeably worse code is still generated, but I couldn’t exactly pinpoint why.

cc @alexcrichton #32204

@eefriedman
Copy link
Contributor

Maybe it's a problem that EncodeUtf8::next() isn't marked #[inline]? Or are you not running into that issue because you're writing code inside libstd?

@nagisa
Copy link
Member Author

nagisa commented Jul 28, 2016

Yes, I was writing code inside libstd (libcore, rather), so the method got inlined regardless of attribute (or lack of it thereof).

@eefriedman
Copy link
Contributor

I looked into this a bit; there are a few related problems, but I think the biggest problem is that LLVM can't really figure out that accesses to EncodeUtf8.buf don't alias accesses to EncodeUtf8.pos, which prevents a lot of simplifications. There's been some discussion of this on the LLVM mailing lists; see http://lists.llvm.org/pipermail/llvm-dev/2016-July/102867.html and http://lists.llvm.org/pipermail/llvm-dev/2016-July/102472.html .

@nagisa
Copy link
Member Author

nagisa commented Mar 12, 2017

Not relevant anymore as a different approach is now used, that generates pretty nice code.

@nagisa nagisa closed this as completed Mar 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants