Merge pull request #16499 from argotorg/mark-on-enqueue-for-bring-up-target-slot-stack-traversal

Stack shuffler: use mark-on-enqueue for bringUpTargetSlot

Author: clonker

Changelog.md

@@ -7,6 +7,7 @@ Compiler Features:
 * General: Restrict the existing experimental features (`generic-solidity`, `lsp`, `ethdebug`, `eof`, `evm`, `ast-import`, `evmasm-import`, `ir-ast`, `ssa-cfg`) to experimental mode.
 * Metadata: Store the state of the experimental mode in JSON and CBOR metadata. In CBOR this broadens the meaning of the existing `experimental` field, which used to indicate only the presence of certain experimental pragmas in the source.
 * Standard JSON Interface: Introduce `settings.experimental` setting required for enabling the experimental mode.
+* Yul EVM Code Transform: Improve stack shuffler performance by fixing a BFS deduplication issue.
 
 Bugfixes:
 

libyul/backends/evm/StackHelpers.h

@@ -186,13 +186,13 @@ class Shuffler
 	static bool bringUpTargetSlot(ShuffleOperations& _ops, size_t _targetOffset)
 	{
 		std::list<size_t> toVisit{_targetOffset};
-		std::set<size_t> visited;
+		// mark on enqueue
+		std::set<size_t> visited{_targetOffset};
 
 		while (!toVisit.empty())
 		{
 			auto offset = *toVisit.begin();
 			toVisit.erase(toVisit.begin());
-			visited.emplace(offset);
 			if (_ops.targetMultiplicity(offset) > 0)
 			{
 				_ops.pushOrDupTarget(offset);
@@ -204,11 +204,12 @@ class Shuffler
 					!_ops.isCompatible(nextOffset, nextOffset) &&
 					_ops.isCompatible(nextOffset, offset)
 				)
-					if (!visited.count(nextOffset))
+					if (visited.insert(nextOffset).second)  // insert returns false if already present
 						toVisit.emplace_back(nextOffset);
 		}
 		return false;
 	}
+
 	/// Performs a single stack operation, transforming the source layout closer to the target layout.
 	template<typename... Args>
 	static bool shuffleStep(Args&&... args)

test/libyul/yulOptimizerTests/fullSuite/stack_shuffler_bring_up_target_slot_bfs_dedup.yul

@@ -0,0 +1,83 @@
+// This is a regression test in context of https://github.com/argotorg/solidity/pull/16499:
+// A BFS deduplication performance bug in `bringUpTargetSlot` where the same offset could be enqueued multiple times
+// before being visited, by marking offsets as seen on enqueue rather than on dequeue.
+{
+    mstore(memoryguard(0x10000), 1)
+    sstore(mload(calldataload(0)), 1)
+    {
+        function foo_n_0(x_4, x_5)
+        {
+            if x_5 {
+                x_4 := 0x2000000000001
+            }
+            x_4 := call(
+                call(
+                    blockhash(0x200000000001),
+                    0x20000000001,
+                    0x2000000001,
+                    mod("h", 32768),
+                    mod(0x200000001, 32768),
+                    mod(0x20000001, 32768),
+                    mod(0x2000001, 32768)
+                ),
+                0x200001,
+                0x20001,
+                mod(0x2001, 32768),
+                mod(calldatasize(), 32768),
+                mod(calldatasize(), 32768),
+                mod(
+                    call(
+                        call(
+                            x_4,
+                            0x201,
+                            number(),
+                            mod(0x21, 32768),
+                            mod(0x3, 32768),
+                            mod(0x3f, 32768),
+                            mod(address(), 32768)
+                        ),
+                        0x3ff,
+                        0x3fff,
+                        mod(0x3ffff, 32768),
+                        mod(9042383626829830, 32768),
+                        mod(0x3fffff, 32768),
+                        mod(0x3ffffff, 32768)
+                    ),
+                    32768
+                )
+            )
+        }
+        foo_n_0(0x200000000000000001, 0x20000000000000001)
+        foo_n_0(sload(224),calldataload(288))
+        foo_n_0(0x3ffffffffffff, 0x3fffffffffffff)
+        foo_n_0(0x3fffffffffffffff, 0x3ffffffffffffffff)
+        foo_n_0(0x3ffffffffffffffffff, 0x3fffffffffffffffffff)
+    }
+}
+// ====
+// EVMVersion: >homestead
+// ----
+// step: fullSuite
+//
+// {
+//     {
+//         mstore(memoryguard(0x010000), 1)
+//         sstore(mload(calldataload(0)), 1)
+//         let x := 0x200000000000000001
+//         x := 0x2000000000001
+//         let _1 := and(call(call(x, 0x201, number(), 0x21, 0x3, 0x3f, and(address(), 32767)), 0x3ff, 0x3fff, 32767, 6, 32767, 32767), 32767)
+//         pop(call(call(blockhash(0x200000000001), 0x20000000001, 0x2000000001, and("h", 32767), 1, 1, 1), 0x200001, 0x20001, 0x2001, and(calldatasize(), 32767), and(calldatasize(), 32767), _1))
+//         let x_1 := sload(224)
+//         if calldataload(288) { x_1 := x }
+//         pop(call(call(blockhash(0x200000000001), 0x20000000001, 0x2000000001, and("h", 32767), 1, 1, 1), 0x200001, 0x20001, 0x2001, and(calldatasize(), 32767), and(calldatasize(), 32767), and(call(call(x_1, 0x201, number(), 0x21, 0x3, 0x3f, and(address(), 32767)), 0x3ff, 0x3fff, 32767, 6, 32767, 32767), 32767)))
+//         let x_2 := 0x3ffffffffffff
+//         x_2 := x
+//         pop(call(call(blockhash(0x200000000001), 0x20000000001, 0x2000000001, and("h", 32767), 1, 1, 1), 0x200001, 0x20001, 0x2001, and(calldatasize(), 32767), and(calldatasize(), 32767), and(call(call(x, 0x201, number(), 0x21, 0x3, 0x3f, and(address(), 32767)), 0x3ff, 0x3fff, 32767, 6, 32767, 32767), 32767)))
+//         let x_3 := 0x3fffffffffffffff
+//         x_3 := x
+//         pop(call(call(blockhash(0x200000000001), 0x20000000001, 0x2000000001, and("h", 32767), 1, 1, 1), 0x200001, 0x20001, 0x2001, and(calldatasize(), 32767), and(calldatasize(), 32767), and(call(call(x, 0x201, number(), 0x21, 0x3, 0x3f, and(address(), 32767)), 0x3ff, 0x3fff, 32767, 6, 32767, 32767), 32767)))
+//         let x_4 := 0x3ffffffffffffffffff
+//         x_4 := x
+//         pop(call(call(blockhash(0x200000000001), 0x20000000001, 0x2000000001, and("h", 32767), 1, 1, 1), 0x200001, 0x20001, 0x2001, and(calldatasize(), 32767), and(calldatasize(), 32767), and(call(call(x, 0x201, number(), 0x21, 0x3, 0x3f, and(address(), 32767)), 0x3ff, 0x3fff, 32767, 6, 32767, 32767), 32767)))
+//     }
+// }