Skip to content

Const evaluation #452

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

Open
bjorn3 opened this issue May 26, 2021 · 3 comments
Open

Const evaluation #452

bjorn3 opened this issue May 26, 2021 · 3 comments

Comments

@bjorn3
Copy link

bjorn3 commented May 26, 2021

@philberty you are planning to use gcc's const folding for constant evaluation, right? How will this handle arbitrarily complex code like this:

// This feature gate is only used within from_bytes_with_nul_unchecked.
// The same point stands with from_bytes_with_nul returning a unit and removing from_bytes_with_nul_unchecked.
#![feature(const_raw_ptr_deref)]

struct CStr([u8]);

enum CStrConvertError {
    NotNulTerminated,
    InteriorNul,
}

impl CStr {
    pub const fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, CStrConvertError> {
        if bytes.is_empty() {
            return Err(CStrConvertError::NotNulTerminated);
        }
        if bytes[bytes.len() - 1] != 0 {
            return Err(CStrConvertError::NotNulTerminated);
        }
        let mut i = 0;
        // `i + 1 < bytes.len()` allows LLVM to optimize away bounds checking,
        // while it couldn't optimize away bounds checks for `i < bytes.len() - 1`.
        while i + 1 < bytes.len() {
            if bytes[i] == 0 {
                return Err(CStrConvertError::InteriorNul);
            }
            i += 1;
        }
        // SAFETY: We just checked that all properties hold.
        Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
    }
    
    pub const unsafe fn from_bytes_with_nul_unchecked(bytes: &[u8]) -> &CStr {
        // Note: This can be done using pointer deref (which requires
        // `const_raw_ptr_deref` to be const) or `transmute` (which requires
        // `const_transmute` to be const) or `ptr::from_raw_parts` (which
        // requires `ptr_metadata`).
        // While none of them are current stable, it is very likely that one of
        // them will eventually be.
        &*(bytes as *const [u8] as *const Self)
    }
}

(adapted from Rust-for-Linux/linux#273)

I would expect gcc's const folding to not be able to handle at least the while loop here.

@philberty
Copy link
Member

It will be an interesting case to see how the code comes out. My knowledge of GCC middle end's constant folding/propagation is not enough to say how optimized this can/will be if we get all the GENERIC trees right.

Note GCCRS is already using GCC's constant folding since it's all part of how you create the GENERIC trees like build_fold2() et'al.

@philberty
Copy link
Member

We might be getting close to being able to try running this though our constexpr evaluator maybe end of summer or September all going well we should keep this issue in a project kanban to track it

@CohenArthur
Copy link
Member

CohenArthur commented Sep 13, 2022

Now that @abbasfaisal 's const folder has been merged, I've adapted the testcase to parry our shortocomings (such as not handling indexing into &[u8] yet or other similar errors within the typechecker):

// This feature gate is only used within from_bytes_with_nul_unchecked.
// The same point stands with from_bytes_with_nul returning a unit and removing from_bytes_with_nul_unchecked.
#![feature(const_raw_ptr_deref)]

struct CStr([u8]);

enum CStrConvertError {
    NotNulTerminated,
    InteriorNul,
}

enum Result<T, E> {
    Ok(T),
    Err(E),
}

impl CStr {
    pub const fn from_bytes_with_nul(
        bytes: [u8; 5],
        size: usize,
    ) -> Result<&Self, CStrConvertError> {
        if size == 0 {
            return Result::Err(CStrConvertError::NotNulTerminated);
        }
        if bytes[size - 1] != 0 {
            return Result::Err(CStrConvertError::NotNulTerminated);
        }
        let mut i = 0;
        // `i + 1 < bytes.len()` allows LLVM to optimize away bounds checking,
        // while it couldn't optimize away bounds checks for `i < bytes.len() - 1`.
        while i + 1 < size {
            if bytes[i] == 0 {
                return Result::Err(CStrConvertError::InteriorNul);
            }
            i += 1;
        }
        // SAFETY: We just checked that all properties hold.
        Result::Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
    }

    pub const unsafe fn from_bytes_with_nul_unchecked(bytes: [u8; 5]) -> &CStr {
        // Note: This can be done using pointer deref (which requires
        // `const_raw_ptr_deref` to be const) or `transmute` (which requires
        // `const_transmute` to be const) or `ptr::from_raw_parts` (which
        // requires `ptr_metadata`).
        // While none of them are current stable, it is very likely that one of
        // them will eventually be.
        &*(&bytes as &[u8] as *const [u8] as *const Self)
    }
}

const BYTES: [u8; 5] = [0x77, 'E' as u8, 'L' as u8, 'F' as u8, 0];
const STR: Result<&CStr, CStrConvertError> = CStr::from_bytes_with_nul(BYTES, 5);

This actually goes through the loop properly, failing with the following error:

test.rs:31:9: error: ‘constexpr’ loop iteration count exceeds limit of 262144 (use ‘-fconstexpr-loop-limit=’ to increase the limit)
   31 |         while i + 1 < size {
      |         ^

(The code is EXTREMELY wrong and unsafe but it passes our typechecker and forces our constchecker to run. Namely, the cast chain in from_bytes_with_nul_unchecked is extremely invalid)

So I think the folder in itself will be okay, but we're certainly having an issue when going through the loop. We recently had another one which was sort of similar: #1523

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

3 participants