✨ Include constant int segments in interference analysis

[fruno-bulax]

Closes #5013
Closes #5126

This PR extends jak's interference analysis to integer segments. The tricky bit was getting the right bit-representation for integers, plugging it into the analysis seemed to have just worked ✨

One hitch is that, unlike with strings, you can have ludicrously large literal integer segments without typing them out. Take for example this lovely test that tried to allocate a petabyte of memory to represent the segment.

#[test]
fn javascript_unsafe_int_segment_size_in_pattern() {
    assert_js_warning!(
        r#"
pub fn go() {
  let assert <<0:9_007_199_254_740_992>> = <<>>
}
"#
    );
}

To avoid this, I included an arbitrary limit of roughly 8kb beyond which I don't produce the bit pattern.

[giacomocavalieri]

Maybe I'm not understanding this properly but shouldn't it be impossible to have a Greater here given the previous checks?

[fruno-bulax]

The bytes size may still be larger because it operates on bytes.

For example, for the number 3 we will have the bytes [0b0000_0011]. If we have a read size of three, we'd need to slice off the first five bits!

[giacomocavalieri]

Makes sense, thank you! Could you add this as a comment to have as an explanation for those who might wonder the same thing in future?

[fruno-bulax]

This was technically explained in the little wall of text, but I've reworded it into more digestible chunks at the corresponding match arms.

[giacomocavalieri]

Could you use the full name here instead of the single letter?

Suggested change

	Some(r) => bits >= r,
	Some(required_bits) => bits >= required_bits,

[fruno-bulax]

Done.

[giacomocavalieri]

I'm not sure this comment is right, if a number is signed but positive wouldn't the most significant bit be 0 in that case? Isn't it 1 only when a signed number is negative? And the test is checking that the number is indeed positive.

Suggested change

	// The minimal representation of an unsigned number always has 1 as its most
	// significant bit and thus always needs an extra bit for the sign.
	bits_unsigned + 1
	// The minimal representation of an unsigned number always needs 1
	// extra bit for the sign.
	bits_unsigned + 1

[fruno-bulax]

I don't think I understand this comment. Did you misread my comment as signed rather than unsigned or am I misunderstanding you?

1. In this branch, the number is signed and non-negative.
2. BigInt::bits returns the minimum number of bits required to represent the number not considering the sign. Or in other words, the number of bits required to represent the absolute value as an unsigned integer.
3. For positive numbers, the signed representation needs the 0 sign bits.
4. For zero, BigInt::bits returns 0, so we also need to add 1. I failed to make this clear in the implementation.

I agree the implementation is rather confusing! Do you think putting it in a big 'ol match expression makes it clearer?

#[must_use]
fn representable_with_bits(value: &BigInt, bits: u32, signed: bool) -> bool {
    // No number is representable in 0 bits.
    if bits == 0 {
        return false;
    };

    let required_bits = match (value.sign(), signed) {
        // Zero always needs one bit.
        (Sign::NoSign, _) => 1,

        // `BigInt::bits` does not consider the sign!
        (Sign::Plus, false) => value.bits(),
        // Therefore we need to add the sign bit here: `10` -> `010`
        (Sign::Plus, true) => value.bits() + 1,

        // A negative number must be signed
        (Sign::Minus, false) => return false,
        (Sign::Minus, true) => {
            let bits_unsigned = value.bits();
            let trailing_zeros = value
                .trailing_zeros()
                .expect("trailing_zeros to return a value for non-zero numbers");
            let is_power_of_2 = trailing_zeros == bits_unsigned - 1;

            // Negative powers of two don't need an extra sign bit. E.g. `-2 == 0b10`
            if is_power_of_2 {
                bits_unsigned
            } else {
                bits_unsigned + 1
            }
        }
    };

    match required_bits.to_u32() {
        Some(required_bits) => bits >= required_bits,
        None => false,
    }
}

[giacomocavalieri]

This looks lovely, yeah I think it's a lot easier to understand!

[fruno-bulax]

pattern matching OP as always.

[giacomocavalieri]

Could you turn this into an if expression rather than converting the bool to an integer? Would make the code easier to understand I think:

Suggested change

	let is_power_of_2 = trailing_zeros == bits_unsigned - 1;
	bits_unsigned + (!is_power_of_2 as u64)
	let is_power_of_2 = trailing_zeros == bits_unsigned - 1;
	if is_power_of_2 { ... } else { ... }

[fruno-bulax]

Done!

[giacomocavalieri]

Looks great! I've left a couple of nits and one question regarding the implementation!

[fruno-bulax]

I've rebased the PR and addressed the open issues; There's a failing check due to a deprecated image that gears is fixing in #5130