Number often uses to_u1024_bytes for bitwise/arithmetic operations, but u1024 has 128bytes while Number is up to 1024bytes

[6293]

https://github.com/AluVM/rust-aluvm/blob/2bde05b47e10fc0a06eb12c868a14dbeff1551ec/src/data/bitwise.rs#L50-L57

https://github.com/AluVM/rust-aluvm/blob/97bd8e7c3e95093a9bf5960b82775f39df3ebeed/src/data/arithm.rs#L128-L133

[6293]

e.g. Shl/Shr is also applicable to r2048 reg

[dr-orlovsky]

It is true, but it is just a sideffect. The only solution to this is to have a different Number types for A and R registers. There is no point in SHL/SHR and other arithmetic operations for R register since they are used to represent hashes/curve points and can't be operated with normal arithmetics.

[6293]

There is no point in SHL/SHR and other arithmetic operations for R register

Unlike bitwise operations like SHL/SHR, arithmetic operation does not exist for R register. Instead of introducing another Number type for R-reg, can we stop using Number on bitwise operations for R-reg?

[6293]

because it is relatively easier to perform bitwise operations on a byte array, than arithmetic operations

[dr-orlovsky]

I think this is a very good idea!

[dr-orlovsky]

The change is quite complex: it requires changing multiple APIs which abstract a specific register type and allow to operate with any A, R or F register providing Number/MaybeNumber.

The change is also an API-breaking

[6293]

We we need to do is to change CoreRegs::get so that it accepts impl Into<RegAF> instead of impl Into<RegAFR>. When one wants to get R-reg value, they should use CoreRegs::get_r instead

[dr-orlovsky]

Agree, that is the best way forward. Can you work on that please?

[6293]

Do we stop using MaybeNumber on PutOp::PutR and introduce something like ByteArray? In this scenario, Number is now nothing to do with R-reg, so maybe Number should internally be [u8; 128] instead of [u8; 1024]. This is a breaking change, but no changes to bytecode.

[dr-orlovsky]

It's ok to have API breaking changes - the bytecode breakes (or business logic in instruction execution) is much much worse.

I think your proposal is a good solution.

[6293]

Hi, hasn't been active for a while. I am facing a challenge regarding with the solution above. Taking MoveOp::MovR for example:

MoveOp::MovR(reg, idx1, idx2) => {
    regs.get_r_mut(*reg, idx2).copy_from_slice(regs.get_r(*reg, idx1));
}

This will not compile because it borrows regs as immutable while also borrowing as mutable. We cannot indicate on a method signature that only a subset of the struct is borrowed. One way to solve this problem is that just expose registers across crate. Alternatively we can have similar getter/setter as RegAF registers, which creates a copy of register value instead of returning reference. With this approach, however, I am afraid that copying of large array occurs too often.

[dr-orlovsky]

I think we just need to clone the result of regs.get_r here - that will be much simplier. Not perfect, but it will make refactoring reasonably-sized.

[6293]

A heap allocation is inevitable when cloning a slice. I thought we should ensure that a program would run without any memory allocation except at VM startup time, are we ok with that?

[dr-orlovsky]

The point was to avoid large memory allocations which may affect low-end hardware (like 64kB+ etc).

Even now AluVM does a lot of allocations of a smaller size.

[dr-orlovsky]

I mean it would be great to avoid any allocations runtime, but I think this is the larger goal than the scope of this issue and it will require more refactoring, so should be left for the future.

[dr-orlovsky]

Fulfilled with v0.12