chore: Document requirement for range opcode on r_witness in GeneratedAcir::euclidean_division

[TomAFrench]

Description

Problem*

We currently emit two range opcodes for r_witness here where the second is always more strict than the first.

Summary*

This PR removes the first range opcode emitted for r_witness in euclidean_division as r < rhs < 2^{rhs_max_bits} can be relaxed to r < rhs without affecting the constraints on r.

The restrictions on r_witness in euclidean_division are a little misleading as in a lot of cases they result in duplicated range opcodes, e.g.

BLACKBOX::RANGE [(_7, num_bits: 32)] [ ] // q_witness
BLACKBOX::RANGE [(_8, num_bits: 33)] [ ] // r_witness
BLACKBOX::RANGE [(_8, num_bits: 32)] [ ] // r_witness

This duplication comes from an optimization from inside bound_constraint_with_offset which isn't guaranteed to happen (but does for a large number of cases). This PR documents this so that people don't go around trying to remove it in future based on ACIR which looks like it's unnecessary.

Documentation

• This PR requires documentation updates when merged.

  • I will submit a noir-lang/docs PR.
  • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

[TomAFrench]

Bump on this. I think this should be fairly uncontroversial

[guipublic]

Bound constraints work on inputs having a specified bit size, which is given in argument to the function. This is why the range constraint on r_witness is important and cannot be removed.

[TomAFrench]

I'm seeing two range opcodes being emitted for the same witness here. Looking again at this we're creating new witnesses in bound_constraint_with_witness so maybe I'm not making the change I want to.

[TomAFrench]

Seems like we're getting this duplicated range opcode due to euclidean division calling bound_constraint_with_offset with with a constant rhs which is a round power of two. This results in aor = lhs + 0 = lhs so

let witness = self.get_or_create_witness(&aor);
self.range_constraint(witness, bit_size)?;

then gets simplified to creating a new range constraint on lhs (r_witness) instead of creating a new witness.

Looks like we could make this optimization but I think relying on another optimization to make it safe feels a bit iffy. Especially as ACVM will optimize out the redundant opcode anyway. Euclidean division isn't guaranteed to use a denominator of a power of 2, it's just the most common usage of euclidean division in ACIR does.

[TomAFrench]

I'm going to add this range constraint back in and add a little documentation instead.