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

Optimize tvec::iter_vec_raw #3729

Closed
catamorphism opened this issue Oct 11, 2012 · 5 comments · Fixed by #21115
Closed

Optimize tvec::iter_vec_raw #3729

catamorphism opened this issue Oct 11, 2012 · 5 comments · Fixed by #21115
Labels
A-codegen Area: Code generation I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@catamorphism
Copy link
Contributor

FIXME in tvec::iter_vec_raw says:

FIXME (#2536): Optimize this when the size of the unit type is // statically known to not use pointer casts, which tend to confuse // LLVM.
#2536 is now closed, so filing this separately.

@graydon
Copy link
Contributor

graydon commented Jun 19, 2013

So this dates from c9c5ee2 (aug 2011) when we had runtime-dynamic-sized types floating around and potentially in the inside of vectors. So the scary part of it is gone. As a minor improvement to codegen on vector glue, one might as well try to implement it properly, which just means switching from byte-adding operations on byte-pointers to GEPi on properly typed pointers.

One could also reasonably argue that this should long since have been moved into library code and/or lang items. I would not object to that solution!

@graydon
Copy link
Contributor

graydon commented Jun 19, 2013

I'd also nominate for removing this from the production-ready milestone. It's not important enough to block. The codegen is so much worse elsewhere, this isn't a high priority.

@pnkfelix
Copy link
Member

just-a-performance-bug, removing milestone.

@pnkfelix
Copy link
Member

visiting for triage, email from 2013 sep 16.

I'm not an expert in the trans code (this is code from middle::trans::tvec). Its not obvious to me what is blocking this bug, if anything. But fixing it locally within iter_vec_raw seems like it is not possible, not at least without changing its interface (because I would not want to emit code here to convert from the provided length-in-bytes to the necessary vec-elem-count).

@flaper87
Copy link
Contributor

Visiting for triage

Work is still needed: iter_vec_raw

dotdash added a commit to dotdash/rust that referenced this issue Jan 13, 2015
There are two places left where we used to only know the byte
size of/offset into an array and had to cast to i8 and back to get the
right addresses. But by now, we always know the sizes in terms of the
number of elements in the array. In fact we have to add an extra Mul
instruction so we can use the weird cast-to-u8 code. So we should really
just embrace our new knowledge and use simple GEPs to do the address
calculations.

Additionally, the pointer calculations in bind_subslice_pat don't handle
zero-sized types correctly, producing slices that point outside the
array that is being matched against. Using GEP fixes that as well.

Fixes rust-lang#3729
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 15, 2015
There are two places left where we used to only know the byte
size of/offset into an array and had to cast to i8 and back to get the
right addresses. But by now, we always know the sizes in terms of the
number of elements in the array. In fact we have to add an extra Mul
instruction so we can use the weird cast-to-u8 code. So we should really
just embrace our new knowledge and use simple GEPs to do the address
calculations.

Fixes rust-lang#3729
bors pushed a commit to rust-lang-ci/rust that referenced this issue May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants