chore: simplify signed shift right

[asterite]

Description

Problem

No issue, just something I didn't fully understand while greenlighting remove_bit_shifts.

Summary

When simplifying a signed shift right the SSA we produced was:

• cast lhs to Field
• add one to it, if it's negative
• truncate it to the original type
• cast it back to the original type
• perform the division
• subtract one to it, if it's negative
• truncate it

I noticed two things:

1. I didn't understand why going to Field to perform the first operation was necessary
2. I didn't understand why the final truncation was needed, if it's truncating to N bits a value that has a maximum value of N bits

I finally understood why the final truncation is needed (and also the intermediate one). I documented this in the Truncate instruction. I'm still not sure it's intuitive (I feel there's a leaky abstraction here) but at least now it's documented.

Then, going to Field isn't necessary because truncating already takes care of doing it the right way. This doesn't result in any ACIR improvement but at least it's less Rust code and less SSA instructions.

Additional Context

Documentation

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: dab424f	Previous: 569ce5f	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	278 s	207 s	1.34
test_report_zkpassport_noir_rsa_	1 s	0 s	+∞

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[asterite]

I'm thinking the alternative would be to still use Field for this operations. What was missing is also doing the subtraction using Field so it would wrap around, then truncating the Field value. That way we could also try simplifying truncate v0, bit_size: 8 when v0 has 8 bits or less.

Thoughts?

[asterite]

Or maybe nevermind the above, I noticed we do this tricky in a few places already.

[TomAFrench]

I don't think this is correct. It's commented above that this cannot overflow so we're just using an unchecked operation to avoid an unnecessary range check.

If shifted_complement == 0 and lhs_sign_as_int == 1 as required for this to overflow then we'd get 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000 as the result which would not truncate into a signed type correctly.

[TomAFrench]

I'm not a fan of us allowing values to escape the bounds implied by their type as we could have logic elsewhere in the compiler making assumptions based on type information which this would then break.

Casting to Field in this case ensures that this does not happen and is safer.

[asterite]

In expand_signed_checks we also perform some math in signed types, then truncate to the max bit size (which in my mind should be a no-op). I tried casting to Field there too before doing the math but it doesn't seem to work or to be a straight-forward change, so for now I'll drop this PR.