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

The introduction of a locally known loop breaks manual assertions #124979

Open
c410-f3r opened this issue May 10, 2024 · 1 comment
Open

The introduction of a locally known loop breaks manual assertions #124979

c410-f3r opened this issue May 10, 2024 · 1 comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@c410-f3r
Copy link
Contributor

c410-f3r commented May 10, 2024

use std::ptr;

pub struct BytesVector {
  vec: Vec<u8>,
}

impl BytesVector {
  #[inline(always)] // Doesn't work without 'always'
  fn reserve(&mut self, additional: usize) {
    self.vec.reserve(additional);
    unsafe {
      if self.vec.len().unchecked_add(additional) > self.vec.capacity() {
        std::hint::unreachable_unchecked();
      }
    }
  }

  fn push(&mut self, value: u8) {
    if self.vec.len() >= self.vec.capacity() {
      panic!();
    }
    unsafe {
      ptr::write(self.vec.as_mut_ptr().add(self.vec.len()), value);
      let new_len = self.vec.len().unchecked_add(1);
      self.vec.set_len(new_len);
    }
  }
}

pub fn push(v: &mut BytesVector) {
    v.reserve(4);
    v.push(0); // No branch
    v.push(1); // No branch
    v.push(2); // No branch
    v.push(3); // No branch
}

The above snippet successfully removes panicking branches due to unreachable_unchecked but the same thing can not be said when working with loops that contain a locally known stopping criteria.

pub fn push(v: &mut BytesVector) {
    v.reserve(4);
    for idx in 0..4 {
       v.push(idx);  // Branches everywhere
    }
}

https://godbolt.org/z/b7hTb9ahG

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 10, 2024
@saethlin saethlin added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 12, 2024
@c410-f3r
Copy link
Contributor Author

Pinging the members that are more involved with codegen to create awareness. Let me known if I can somehow help in this matter.

@nikic @bjorn3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants