Add simplification rule for bitwise shifting#3580
Conversation
d78fc1f to
255f7a5
Compare
| u256 mask = (u256(1) << testBit) - 1; | ||
| return u256(boost::multiprecision::bit_test(B.d(), testBit) ? B.d() | ~mask : B.d() & mask); | ||
| }, false}, | ||
| {{Instruction::SHL, {A, B}}, [=]{ |
There was a problem hiding this comment.
@chriseth do we want to introduce evmVersion in the rule list or is it safe to have this merged (after some cleanup/squashing)?
There was a problem hiding this comment.
I think it should be safe. Let's add a comment.
There was a problem hiding this comment.
Are you sure about the order of the operands?
There was a problem hiding this comment.
This is blocked by the same problem as in #3797: need to build a constantinople supporting version of eth and run endtoend tests with that target.
There was a problem hiding this comment.
Are you sure about the order of the operands?
The top of the stack is the number of bits to shift he value with.
|
@pirapira can you check this, since you have written tests for shifts? |
| {{Instruction::SHR, {A, B}}, [=]{ | ||
| if (A.d() > 255) | ||
| return u256(0); | ||
| return B.d() >> unsigned(A.d()); |
There was a problem hiding this comment.
Is B.d() signed or unsigned? If signed, can it be negative?
There was a problem hiding this comment.
SHR always considers B as unsigned. There is no optimisation rule adde for SAR as I was lazy.
9685f89 to
2e73ef5
Compare
chfast
left a comment
There was a problem hiding this comment.
For constants this is ok.
What was the other one what tried to optimize division?
Depends on #2541.