chore(interpreter): optimisation for BYTE, SHL, SHR and SAR #1418
Conversation
| }; | ||
| let o1 = as_usize_saturated!(op1) % 32; | ||
| let byte_value = op2.byte(31 - o1); | ||
| *op2 = U256::from(byte_value); |
There was a problem hiding this comment.
I don't think this is right, but also not sure if there is a test for this in statetests
There was a problem hiding this comment.
This way when 'o1' exceeds 31, the function returns ZERO but to ensure the implementation works as expected, can you suggest any specific tests we should do just in case?
There was a problem hiding this comment.
I don't think so? 33 % 32 = 1 so it will be 31 - 1 byte instead of zero
There was a problem hiding this comment.
Yes, you're right. Sometimes we have things right in front of our eyes and they escape us. If you have any ideas on how to improve the method, please let me know, I am going to continue thinking about it ^^
There was a problem hiding this comment.
we can shifts the op2 right by calculated bits to align and isolate the desired byte using bitmask but I don't think it's better than byte(..)
something like this:
*op2 = if o1 < 32 {
let shift = (31 - o1) * 8;
U256::from((*op2 >> shift) & U256::from(0xFF))
} else {
U256::ZERO
};What do you think ?
There was a problem hiding this comment.
I believe byte is already optimal on since it will simply index into the value as a byte array. Thanks for the efforts anyway.
There was a problem hiding this comment.
Thanks, I'm going to leave this method as it was =)
| } | ||
|
|
||
| /// EIP-145: Bitwise shifting instructions in EVM | ||
| pub fn shr<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, _host: &mut H) { | ||
| check!(interpreter, CONSTANTINOPLE); | ||
| gas!(interpreter, gas::VERYLOW); | ||
| pop_top!(interpreter, op1, op2); | ||
| *op2 >>= as_usize_saturated!(op1); | ||
| let shift = as_usize_saturated!(op1); | ||
| *op2 = if shift < 256 { |
There was a problem hiding this comment.
I was looking at what ruint do
https://github.com/recmo/uint/blob/b041f09be7035bb61f8e52e39194eb838e832483/src/bits.rs#L353-L356
shift < 256 is better.
There was a problem hiding this comment.
Nice. so, are we on the right track?
Hey guys, this is my first contribution here, so nice to meet you. I was working on this issue 1251
Motivation:
Investigate the performance of the BYTE, SHL, SHR and SAR OPCODEs and try to improvement
Solution:
The improvements I've tried to apply in the resulting assembly were:
I'm going to attach the before and after assembly for the opcode functions.
Byte
Before
After
SAR
Before
After
SHL
Before
After
SHR
Before
After