From 76c0109b429bc45754e12f38878f03c6142504c1 Mon Sep 17 00:00:00 2001 From: Cyrill Leutwiler Date: Sat, 22 Jul 2023 14:56:06 +0200 Subject: [PATCH] Bugfix: Strength reduce must not optimize mul into shl if overflow (#1452) Optimizing multiplications with a SHL must not disable overflow checks. Signed-off-by: Cyrill Leutwiler --- src/codegen/strength_reduce/mod.rs | 57 ++++++++++--------- .../solidity/strength_reduce.sol | 13 ++++- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/codegen/strength_reduce/mod.rs b/src/codegen/strength_reduce/mod.rs index 10f48c0f6..2ff9ba676 100644 --- a/src/codegen/strength_reduce/mod.rs +++ b/src/codegen/strength_reduce/mod.rs @@ -230,38 +230,41 @@ fn expression_reduce(expr: &Expression, vars: &Variables, ns: &mut Namespace) -> let left_values = expression_values(left, vars, ns); let right_values = expression_values(right, vars, ns); - if let Some(right) = is_single_constant(&right_values) { - // is it a power of two - // replace with a shift - let mut shift = BigInt::one(); - let mut cmp = BigInt::from(2); - - for _ in 1..bits { - if cmp == right { - ns.hover_overrides.insert( - *loc, - format!( - "{} multiply optimized to shift left {}", - ty.to_string(ns), - shift - ), - ); + match is_single_constant(&right_values) { + Some(right) if *overflowing => { + // is it a power of two + // replace with a shift + let mut shift = BigInt::one(); + let mut cmp = BigInt::from(2); + + for _ in 1..bits { + if cmp == right { + ns.hover_overrides.insert( + *loc, + format!( + "{} multiply optimized to shift left {}", + ty.to_string(ns), + shift + ), + ); - return Expression::ShiftLeft { - loc: *loc, - ty: ty.clone(), - left: left.clone(), - right: Box::new(Expression::NumberLiteral { + return Expression::ShiftLeft { loc: *loc, ty: ty.clone(), - value: shift, - }), - }; + left: left.clone(), + right: Box::new(Expression::NumberLiteral { + loc: *loc, + ty: ty.clone(), + value: shift, + }), + }; + } + + cmp *= 2; + shift += 1; } - - cmp *= 2; - shift += 1; } + _ => (), // SHL would disable overflow check } if ty.is_signed_int(ns) { diff --git a/tests/codegen_testcases/solidity/strength_reduce.sol b/tests/codegen_testcases/solidity/strength_reduce.sol index 3fdbedb54..433b65ce7 100644 --- a/tests/codegen_testcases/solidity/strength_reduce.sol +++ b/tests/codegen_testcases/solidity/strength_reduce.sol @@ -43,10 +43,17 @@ contract test { // BEGIN-CHECK: test::function::f4 function f4() pure public { for (uint i = 0; i < 10; i++) { - // this multiply can be done with a 64 bit instruction - print("i:{}".format(i * 32768)); + unchecked { + // this multiply can be done with a 64 bit instruction + print("i:{}".format(i * 32768)); + } } // CHECK: (%i << uint256 15) + for (uint i = 0; i < 10; i++) { + // Do not disable overflow checks + print("i:{}".format(i * 32768)); + } +// NOT CHECK: (%i << uint256 15) } // BEGIN-CHECK: test::function::f5 @@ -114,6 +121,6 @@ contract test { for (int i = 0; i != 102; i++) { print("i:{}".format(i % 0x1_0000_0001)); } -// CHECK: (signed modulo %i % int256 4294967297) + // CHECK: (signed modulo %i % int256 4294967297) } }