From 556bd9adfc05dd7d11fde73842e4a2e6002b9f01 Mon Sep 17 00:00:00 2001
From: Daniel Kirchner <daniel@ekpyron.org>
Date: Fri, 13 Aug 2021 14:33:10 +0200
Subject: [PATCH] Remove fixStackTooDeep and adjust combineStack comments.

---
 libyul/backends/evm/StackLayoutGenerator.cpp | 21 ++++++--------------
 libyul/backends/evm/StackLayoutGenerator.h   |  4 ----
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/libyul/backends/evm/StackLayoutGenerator.cpp b/libyul/backends/evm/StackLayoutGenerator.cpp
index b774b64ac..3dfff88b7 100644
--- a/libyul/backends/evm/StackLayoutGenerator.cpp
+++ b/libyul/backends/evm/StackLayoutGenerator.cpp
@@ -302,7 +302,6 @@ void StackLayoutGenerator::processEntryPoint(CFG::BasicBlock const& _entry)
 	}
 
 	stitchConditionalJumps(_entry);
-	fixStackTooDeep(_entry);
 }
 
 optional<Stack> StackLayoutGenerator::getExitLayoutOrStageDependencies(
@@ -446,8 +445,9 @@ void StackLayoutGenerator::stitchConditionalJumps(CFG::BasicBlock const& _block)
 
 Stack StackLayoutGenerator::combineStack(Stack const& _stack1, Stack const& _stack2)
 {
-	// TODO: there is probably a better way than brute-forcing. This has n! complexity or worse, so
-	// we can't keep it like this.
+	// TODO: it would be nicer to replace this by a constructive algorithm.
+	// Currently it uses a reduced version of the Heap Algorithm to partly brute-force, which seems
+	// to work decently well.
 
 	Stack commonPrefix;
 	for (auto&& [slot1, slot2]: ranges::zip_view(_stack1, _stack2))
@@ -478,13 +478,6 @@ Stack StackLayoutGenerator::combineStack(Stack const& _stack1, Stack const& _sta
 
 	std::map<size_t, Stack> sortedCandidates;
 
-	// TODO: surprisingly this works for rather comparably large candidate size, but we should probably
-	// set up some limit, since this will quickly explode otherwise.
-	// Ideally we would then have a better fallback mechanism - although returning any naive union of both stacks
-	// like ``candidate`` itself may just be fine.
-	//	if (candidate.size() > 8)
-	//		return candidate;
-
 	auto evaluate = [&](Stack const& _candidate) -> size_t {
 		size_t numOps = 0;
 		Stack testStack = _candidate;
@@ -518,6 +511,9 @@ Stack StackLayoutGenerator::combineStack(Stack const& _stack1, Stack const& _sta
 				std::swap(candidate[c[i]], candidate[i]);
 			sortedCandidates.insert(std::make_pair(evaluate(candidate), candidate));
 			++c[i];
+			// Note that for a proper implementation of the Heap algorithm this would need to revert back to ``i = 1.``
+			// However, the incorrect implementation produces decent result and the proper version would have n!
+			// complexity and is thereby not feasible.
 			++i;
 		}
 		else
@@ -530,11 +526,6 @@ Stack StackLayoutGenerator::combineStack(Stack const& _stack1, Stack const& _sta
 	return commonPrefix + sortedCandidates.begin()->second;
 }
 
-void StackLayoutGenerator::fixStackTooDeep(CFG::BasicBlock const&)
-{
-	// TODO
-}
-
 Stack StackLayoutGenerator::compressStack(Stack _stack)
 {
 	optional<size_t> firstDupOffset;
diff --git a/libyul/backends/evm/StackLayoutGenerator.h b/libyul/backends/evm/StackLayoutGenerator.h
index 4555724eb..6e374a767 100644
--- a/libyul/backends/evm/StackLayoutGenerator.h
+++ b/libyul/backends/evm/StackLayoutGenerator.h
@@ -85,10 +85,6 @@ private:
 	/// stack shuffling when starting from the returned layout.
 	static Stack combineStack(Stack const& _stack1, Stack const& _stack2);
 
-	/// Tries to detect stack layout transitions that are bound to cause stack too deep errors and
-	/// attempts to reorganize the layout to avoid those cases.
-	void fixStackTooDeep(CFG::BasicBlock const& _entry);
-
 	/// @returns a copy of @a _stack stripped of all duplicates and slots that can be freely generated.
 	/// Attempts to create a layout that requires a minimal amount of operations to reconstruct the original
 	/// stack @a _stack.