Implement checked exponentiation. #9479

chriseth · 2020-07-22T16:23:37Z

Fixes #9110.
Fixes #8853.

chriseth · 2020-07-22T16:35:44Z

Would be nice to know the gas costs of this.

chriseth · 2020-07-23T15:37:35Z

The inner loop itself takes about 144 gas per iteration. For an exp that uses the full 8 iterations this is about 1200 gas per exp operation.

chriseth · 2020-07-23T15:43:23Z

In comparison, the opcode itself costs 10 gas.

hrkrshnn · 2020-07-24T14:18:39Z

If the LHS of the operation is a Literal, can we write a version where the compiler computes the max-value of the exponent: 256/log_2(integer_literal) The code reverts during runtime if the exponent is greater than or equal to this value. Otherwise, we make a call to exp (or shl if base is 2)

I tried to look at all the non-trivial examples (i.e., not literal**literal) of exponentiation. All such cases had a literal as a base, mostly 2, some 10, one 256. Here is a list of use cases https://gist.github.com/hrkrshnn/c73874b150d4a2c1ec6ebbc345efa9b3

leonardoalt

I think @hrkrshnn 's idea should be implemented.
@chriseth what do you think?

leonardoalt · 2020-07-27T09:04:57Z

test/libsolidity/semanticTests/cleanup/exp_cleanup_smaller_base.sol

@@ -0,0 +1,11 @@
+contract C {


Wasn't this test added in some other PR? Also, why not also via Yul?

leonardoalt · 2020-07-27T09:06:21Z

libsolidity/codegen/YulUtilFunctions.cpp

+{
+	solUnimplementedAssert(!_type.isSigned(), "");
+	solAssert(!_exponentType.isSigned(), "");
+	string functionName = "checked_exp_" + _type.identifier();


Does _exponentType need to be in the function name identifier as well?

Good catch!

leonardoalt · 2020-07-27T09:12:33Z

libsolidity/codegen/ExpressionCompiler.cpp

+	solAssert(!dynamic_cast<IntegerType const&>(_exponentType).isSigned(), "");
+
+	m_context << Instruction::EXP;
+


Extra blank

chriseth · 2020-07-27T09:15:46Z

@hrkrshnn @leonardoalt you are right, we should certainly specialize this for literals. it might even make sense to add some conditional to check for the values of non-literals.

chriseth · 2020-07-28T11:56:24Z

After the "optimization for small bases" commit, most of the tests will now not test the loop anymore...

chriseth · 2020-07-28T11:56:36Z

@leonardoalt can we prove this code correct?

chriseth · 2020-07-28T11:58:39Z

All of the examples in @hrkrshnn 's list, the bases were either 2, 10 or 256, so I added optimized version for those bases.

leonardoalt · 2020-07-28T12:05:42Z

@leonardoalt can we prove this code correct?

with a few adjustments I think so

hrkrshnn · 2020-07-28T13:02:50Z

libsolidity/codegen/YulUtilFunctions.cpp

+				// b**e < 2**256 <=> e * log(b) < 256 * log(2) <=> e < 256 * log(2) / log(b)
+				if or(
+					and(lt(base, 11), lt(exponent, 78)),
+					and(lt(base, 257), lt(exponent, 33)),


Suggested change

and(lt(base, 257), lt(exponent, 33)),

and(lt(base, 257), lt(exponent, 32)),

Because 256**32 = 2**256 = UINT256_MAX + 1

hrkrshnn · 2020-07-28T13:13:01Z

libsolidity/codegen/YulUtilFunctions.cpp

+				// small base optimizations
+				if eq(base, 2) {
+					if gt(exponent, 255) { revert(0, 0) }
+					power := exp(2, exponent)


This will be converted to shl by the optimizer right?

We can check, but manly, shl does not exist on all platforms.

Actually it seems we do not have such a rule. I will add it separately.

chriseth · 2020-07-28T19:31:30Z

I just realized that functions containing leave cannot be inlined, which is of course fatal in case the exp function is used together with constants. That is another point in favour of replacing functions with constant arguments by copies. I will rewrite this without leave.

hrkrshnn · 2020-07-29T08:01:21Z

libsolidity/codegen/YulUtilFunctions.cpp

+				base := <cleanupFunction>(base)
+				exponent := <exponentCleanupFunction>(exponent)
+
+				// avoiding `leave` to allow this function to be inlined


Isn't the function too lengthy to be inlined?

libsolidity/codegen/YulUtilFunctions.cpp

axic · 2020-08-05T15:20:07Z

Why do we need checked exponentiation for the yul IR, but not for the old codegen?

chriseth · 2020-08-06T15:09:56Z

This function is tricky. We do not necessarily want it to be inlined, but we want information about the size of the base to be taken into account in the function. Currently, the optimizer can only do this through inlining. Using leave prevents it from being inlined because the concept cannot be adequately replaced in all cases.

In the end of course, we should write the code such that it is best readable and improve the optimizer instead, because Solidity code will also very likely use return.

If a function does not contain a leave inside a loop, we can pack it into for {} 1{} { ... break } and replace the leave by a break, but I am not sure how well the other parts of the optimizer will cope with that.

chriseth · 2020-08-06T15:40:12Z

Reverted to the version that uses leave. Since most of the code is the same regardless of the type, we might consider extracting the part that does not depend on the type into its own function (which then takes max as an explicit parameter), which would reduce code duplication

axic · 2020-08-06T16:54:19Z

libsolidity/codegen/YulUtilFunctions.cpp

+				// 0**0 == 1
+				switch exponent
+				case 0 { power := 1 leave }
+				case 1 { power := base leave }


Ouch, Yul looks weird with these statements on the same line :)

chriseth · 2020-08-17T18:23:58Z

libsolidity/codegen/YulUtilFunctions.cpp

+					if and(exponent, 1)
+					{
+						// no check needed for positive power, because base >= power
+						if and(slt(power, 0), slt(power, sdiv(min, base))) { revert(0, 0) }


I was not able to find a test that does not revert if I comment out this line - maybe the check about base above is sufficient. Maybe someone can prove this superfluous.

chriseth · 2020-08-17T18:25:11Z

I'm now confident this is a version that can be merged, so please review!

The version that optimizes constants and small values will be done in another PR.

hrkrshnn · 2020-08-18T06:30:00Z

libsolidity/codegen/YulUtilFunctions.cpp

+			("maxValue", toCompactHexWithPrefix(u256(_type.maxValue())))
+			("minValue", toCompactHexWithPrefix(u256(_type.minValue())))


Suggested change

("maxValue", toCompactHexWithPrefix(u256(_type.maxValue())))

("minValue", toCompactHexWithPrefix(u256(_type.minValue())))

("maxValue", toCompactHexWithPrefix(_type.max()))

("minValue", toCompactHexWithPrefix(_type.min()))

hrkrshnn · 2020-08-18T07:23:00Z

libsolidity/codegen/YulUtilFunctions.cpp

+					if and(exponent, 1)
+					{
+						// no check needed for positive power, because base >= power
+						if and(slt(power, 0), slt(power, sdiv(min, base))) { revert(0, 0) }


libsolidity/codegen/YulUtilFunctions.cpp

hrkrshnn · 2020-08-18T07:35:54Z

test/libsolidity/semanticTests/viaYul/exp_neg.sol

+// f(int256,uint256): -2, 1 -> -2
+// f(int256,uint256): -2, 2 -> 4
+// f(int256,uint256): -7, 63 -> -174251498233690814305510551794710260107945042018748343
+// f(int256,uint256): -128, 2 -> 0x4000


Can you add this:
// g(int256,uint256): -1, 115792089237316195423570985008687907853269984665640564039457584007913129639935 -> -1

A test case that loops 255 times.

chriseth · 2020-08-18T10:43:40Z

libsolidity/codegen/YulUtilFunctions.cpp

+					// overflow check for base * base
+					if gt(base, div(max, base)) { revert(0, 0) }
+					if and(exponent, 1)
+					{


@hrkrshnn simplified your proof a little - can you check, please?

chriseth requested review from ekpyron and leonardoalt July 22, 2020 16:23

chriseth force-pushed the exp branch from 35ace82 to c05864d Compare July 22, 2020 16:35

chriseth force-pushed the exp branch from c05864d to bb0cef6 Compare July 23, 2020 13:51

chriseth force-pushed the exp branch from bb0cef6 to 7141318 Compare July 23, 2020 16:21

leonardoalt requested changes Jul 27, 2020

View reviewed changes

chriseth force-pushed the exp branch 2 times, most recently from 3e20bc0 to 4a4014c Compare July 28, 2020 11:53

hrkrshnn reviewed Jul 28, 2020

View reviewed changes

chriseth force-pushed the exp branch 3 times, most recently from 33d175e to 1405ed6 Compare July 28, 2020 18:50

chriseth force-pushed the exp branch 3 times, most recently from 87ae687 to 12d0d6a Compare July 28, 2020 20:43

hrkrshnn reviewed Jul 29, 2020

View reviewed changes

hrkrshnn reviewed Aug 3, 2020

View reviewed changes

libsolidity/codegen/YulUtilFunctions.cpp Outdated Show resolved Hide resolved

hrkrshnn reviewed Aug 3, 2020

View reviewed changes

libsolidity/codegen/YulUtilFunctions.cpp Outdated Show resolved Hide resolved

ethereum deleted a comment from stackenbotten Aug 5, 2020

chriseth force-pushed the exp branch from 5d2323c to 45bbb0b Compare August 6, 2020 15:37

ethereum deleted a comment from stackenbotten Aug 6, 2020

axic reviewed Aug 6, 2020

View reviewed changes

axic mentioned this pull request Aug 13, 2020

solidity::util::formatNumberReadable signed integer support #9601

Closed

chriseth force-pushed the exp branch 2 times, most recently from 5ac5d66 to e97d00c Compare August 17, 2020 18:18

ethereum deleted a comment from stackenbotten Aug 17, 2020

chriseth commented Aug 17, 2020

View reviewed changes

chriseth force-pushed the exp branch from e97d00c to 807dc35 Compare August 17, 2020 18:24

hrkrshnn reviewed Aug 18, 2020

View reviewed changes

libsolidity/codegen/YulUtilFunctions.cpp Show resolved Hide resolved

hrkrshnn reviewed Aug 18, 2020

View reviewed changes

chriseth force-pushed the exp branch from 807dc35 to 18a9374 Compare August 18, 2020 10:41

Implement checked exponentiation.

c34e349

chriseth force-pushed the exp branch from 18a9374 to c34e349 Compare August 18, 2020 10:43

chriseth commented Aug 18, 2020

View reviewed changes

hrkrshnn approved these changes Aug 18, 2020

View reviewed changes

chriseth merged commit 17edf4f into develop Aug 18, 2020

chriseth deleted the exp branch August 18, 2020 11:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement checked exponentiation. #9479

Implement checked exponentiation. #9479

chriseth commented Jul 22, 2020 •

edited by axic

Loading

chriseth commented Jul 22, 2020

chriseth commented Jul 23, 2020

chriseth commented Jul 23, 2020

hrkrshnn commented Jul 24, 2020 •

edited

Loading

leonardoalt left a comment

leonardoalt Jul 27, 2020

leonardoalt Jul 27, 2020

chriseth Jul 28, 2020

leonardoalt Jul 27, 2020

chriseth commented Jul 27, 2020

chriseth commented Jul 28, 2020

chriseth commented Jul 28, 2020

chriseth commented Jul 28, 2020

leonardoalt commented Jul 28, 2020

hrkrshnn Jul 28, 2020

hrkrshnn Jul 28, 2020

chriseth Jul 28, 2020

chriseth Jul 28, 2020

chriseth commented Jul 28, 2020

hrkrshnn Jul 29, 2020

axic commented Aug 5, 2020

chriseth commented Aug 6, 2020 •

edited

Loading

chriseth commented Aug 6, 2020

axic Aug 6, 2020

chriseth Aug 17, 2020

hrkrshnn Aug 18, 2020 •

edited

Loading

chriseth commented Aug 17, 2020

hrkrshnn Aug 18, 2020

hrkrshnn Aug 18, 2020 •

edited

Loading

hrkrshnn Aug 18, 2020

chriseth Aug 18, 2020

hrkrshnn Aug 18, 2020

		solAssert(!dynamic_cast<IntegerType const&>(_exponentType).isSigned(), "");

		m_context << Instruction::EXP;

	and(lt(base, 257), lt(exponent, 33)),
	and(lt(base, 257), lt(exponent, 32)),

		("maxValue", toCompactHexWithPrefix(u256(_type.maxValue())))
		("minValue", toCompactHexWithPrefix(u256(_type.minValue())))

Implement checked exponentiation. #9479

Implement checked exponentiation. #9479

Conversation

chriseth commented Jul 22, 2020 • edited by axic Loading

chriseth commented Jul 22, 2020

chriseth commented Jul 23, 2020

chriseth commented Jul 23, 2020

hrkrshnn commented Jul 24, 2020 • edited Loading

leonardoalt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriseth commented Jul 27, 2020

chriseth commented Jul 28, 2020

chriseth commented Jul 28, 2020

chriseth commented Jul 28, 2020

leonardoalt commented Jul 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriseth commented Jul 28, 2020

Choose a reason for hiding this comment

axic commented Aug 5, 2020

chriseth commented Aug 6, 2020 • edited Loading

chriseth commented Aug 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hrkrshnn Aug 18, 2020 • edited Loading

Choose a reason for hiding this comment

chriseth commented Aug 17, 2020

Choose a reason for hiding this comment

hrkrshnn Aug 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriseth commented Jul 22, 2020 •

edited by axic

Loading

hrkrshnn commented Jul 24, 2020 •

edited

Loading

chriseth commented Aug 6, 2020 •

edited

Loading

hrkrshnn Aug 18, 2020 •

edited

Loading

hrkrshnn Aug 18, 2020 •

edited

Loading