[Outlining] Remove overlapping sequences

src/passes/Outlining.cpp

@@ -280,6 +280,8 @@ struct Outlining : public Pass {
     DBG(printHashString(stringify.hashString, stringify.exprs));
     // Remove substrings that are substrings of longer repeat substrings.
     substrings = StringifyProcessor::dedupe(substrings);
+    // Remove substrings with overlapping indices.
+    substrings = StringifyProcessor::filterOverlaps(substrings);
     // Remove substrings with branch and return instructions until an analysis
     // is performed to see if the intended destination of the branch is included
     // in the substring to be outlined.

src/passes/hash-stringify-walker.cpp

@@ -15,6 +15,7 @@
  */
 
 #include "stringify-walker.h"
+#include "support/intervals.h"
 
 namespace wasm {
 
@@ -147,6 +148,48 @@ std::vector<SuffixTree::RepeatedSubstring> StringifyProcessor::dedupe(
   return result;
 }
 
+std::vector<SuffixTree::RepeatedSubstring> StringifyProcessor::filterOverlaps(
+  const std::vector<SuffixTree::RepeatedSubstring>& substrings) {
+  // A substring represents a contiguous set of instructions that appear more
+  // than once in a Wasm binary. For each appearance of the substring, an
+  // Interval is created that lacks a connection back to its originating
+  // substring. To fix, upon Interval creation, a second vector is populated with
+  // the index of the corresponding substring.
+  std::vector<Interval> intervals;
+  std::vector<int> substringIdxs;
+
+  // Construct intervals
+  for (Index i = 0; i < substrings.size(); i++) {
+    auto substring = substrings[i];
+    for (auto startIdx : substring.StartIndices) {
+      auto interval =
+        Interval(startIdx, startIdx + substring.Length, substring.Length);
+      intervals.push_back(interval);
+      substringIdxs.push_back(i);
+    }
+  }
+
+  // Get the overlapping intervals
+  std::vector<SuffixTree::RepeatedSubstring> result;
+  std::vector<std::vector<Index>> startIndices(substrings.size());
+  std::vector<int> indices = IntervalProcessor::filterOverlaps(intervals);
+  for (auto i : indices) {
+    // i is the idx of the Interval in the intervals vector
+    // i in substringIdxs returns the idx of the substring that needs to be
+    // included in result
+    auto substringIdx = substringIdxs[i];
+    startIndices[substringIdx].push_back(intervals[i].start);
+  }
+  for (Index i = 0; i < startIndices.size(); i++) {
+    if (startIndices[i].size() > 1) {
+      result.push_back(SuffixTree::RepeatedSubstring(
+        {substrings[i].Length, std::move(startIndices[i])}));
+    }
+  }
+
+  return result;
+}
+
 std::vector<SuffixTree::RepeatedSubstring> StringifyProcessor::filter(
   const std::vector<SuffixTree::RepeatedSubstring>& substrings,
   const std::vector<Expression*>& exprs,

src/passes/stringify-walker.h

@@ -21,6 +21,7 @@
 #include "ir/module-utils.h"
 #include "ir/stack-utils.h"
 #include "ir/utils.h"
+#include "support/intervals.h"
 #include "support/suffix_tree.h"
 #include "wasm-ir-builder.h"
 #include "wasm-traversal.h"
@@ -264,6 +265,7 @@ using Substrings = std::vector<SuffixTree::RepeatedSubstring>;
 struct StringifyProcessor {
   static Substrings repeatSubstrings(std::vector<uint32_t>& hashString);
   static Substrings dedupe(const Substrings& substrings);
+  static Substrings filterOverlaps(const Substrings& substrings);
   // Filter is the general purpose function backing subsequent filter functions.
   // It can be used directly, but generally prefer a wrapper function
   // to encapsulate your condition and make it available for tests.

src/support/CMakeLists.txt

@@ -7,6 +7,7 @@ set(support_SOURCES
   debug.cpp
   dfa_minimization.cpp
   file.cpp
+  intervals.cpp
   istring.cpp
   json.cpp
   name.cpp

src/support/intervals.cpp

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2024 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <assert.h>
+
+#include "intervals.h"
+#include "support/index.h"
+#include <algorithm>
+#include <iostream>
+#include <optional>
+
+using namespace wasm;
+
+std::vector<int>
+IntervalProcessor::filterOverlaps(std::vector<Interval>& intervals) {
+  if (intervals.size() == 0) {
+    return std::vector<int>();
+  }
+
+  std::vector<std::pair<Interval, int>> intIntervals;
+  for (Index i = 0; i < intervals.size(); i++) {
+    auto& interval = intervals[i];
+    intIntervals.push_back({interval, i});
+  }
+
+  std::sort(
+    intIntervals.begin(), intIntervals.end());
+
+  std::vector<std::pair<Interval, int>> kept;
+  kept.push_back(intIntervals[0]);
+  for (auto& candidate : intIntervals) {
+    auto& former = kept.back();
+    if (former.first.end <= candidate.first.start) {
+      kept.push_back(candidate);
+      continue;
+    }
+
+    // former overlaps with candidate
+    // replace former if the weights are the same but the candidate ends earlier
+    if (former.first.weight == candidate.first.weight && former.first.end > candidate.first.end) {
+      former = candidate;
+    } else if (former.first.weight < candidate.first.weight) {
+      former = candidate;
+    }
+  }
+
+  std::vector<int> result;
+  for (auto& intPair : kept) {
+    result.push_back(intPair.second);
+  }
+
+  return result;
+}

src/support/intervals.h

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2024 WebAssembly Community Group participants
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Helpers for handling a generic range of values
+
+#ifndef wasm_support_intervals_h
+#define wasm_support_intervals_h
+
+#include <set>
+#include <vector>
+
+namespace wasm {
+
+struct Interval {
+  unsigned start;
+  unsigned end;
+  // The weight is used to determine which interval to keep when two overlap,
+  // higher is better
+  unsigned weight;
+  Interval(unsigned start, unsigned end, unsigned weight)
+    : start(start), end(end), weight(weight) {}
+
+  bool operator<(const Interval& other) const {
+    /*return weight != other.weight ? weight < other.weight
+             : end != other.end ? end < other.end
+             : start < other.start;*/
+    /*return end != other.end ? end < other.end
+             : weight != other.weight ? weight < other.weight
+             : start < other.start;*/
+    return start != other.start ? start < other.start
+             : weight != other.weight ? weight < other.weight
+             : end < other.end;
+  }
+
+  bool operator==(const Interval& other) const {
+    return start == other.start && end == other.end && weight == other.weight;
+  }
+};
+
+struct IntervalProcessor {
+  // Given a vector of Interval, returns a vector of the indices that, mapping
+  // back to the original input vector, do not overlap with each other, ie: the
+  // interval indexes with overlapping interval indexes already removed.
+  static std::vector<int> filterOverlaps(std::vector<Interval>&);
+};
+
+} // namespace wasm
+
+#endif // wasm_suport_intervals

test/gtest/CMakeLists.txt

@@ -8,6 +8,7 @@ set(unittest_SOURCES
   dfa_minimization.cpp
   disjoint_sets.cpp
   interpreter.cpp
+  intervals.cpp
   json.cpp
   lattices.cpp
   local-graph.cpp

test/gtest/intervals.cpp

@@ -0,0 +1,121 @@
+#include "support/intervals.h"
+#include "ir/utils.h"
+#include "gtest/gtest.h"
+
+using namespace wasm;
+
+TEST(IntervalsTest, TestEmpty) {
+  std::vector<Interval> intervals;
+  std::vector<int> expected;
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOne) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 2, 5});
+  std::vector<int> expected{0};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestNoOverlapFound) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 4, 2});
+  intervals.emplace_back(Interval{4, 8, 2});
+  std::vector<int> expected{0, 1};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFound) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 2, 5});
+  intervals.emplace_back(Interval{1, 4, 10});
+  intervals.emplace_back(Interval{4, 5, 15});
+  std::vector<int> expected{1, 2};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundA) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 2, 1});
+  intervals.emplace_back(Interval{1, 4, 1});
+  intervals.emplace_back(Interval{4, 6, 1});
+  intervals.emplace_back(Interval{4, 5, 1});
+  std::vector<int> expected{0, 3};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundB) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 3, 4});
+  intervals.emplace_back(Interval{3, 6, 2});
+  intervals.emplace_back(Interval{6, 10, 3});
+  intervals.emplace_back(Interval{8, 11, 1});
+  intervals.emplace_back(Interval{7, 15, 5});
+  std::vector<int> expected{0, 1, 4};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundC) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 3, 6});
+  intervals.emplace_back(Interval{3, 6, 2});
+  intervals.emplace_back(Interval{2, 10, 3});
+  intervals.emplace_back(Interval{8, 11, 1});
+  intervals.emplace_back(Interval{9, 15, 5});
+  intervals.emplace_back(Interval{3, 15, 5});
+  std::vector<int> expected{0, 5};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundD) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{2, 6, 2});
+  intervals.emplace_back(Interval{12, 18, 3});
+  intervals.emplace_back(Interval{4, 11, 1});
+  intervals.emplace_back(Interval{6, 20, 15});
+  intervals.emplace_back(Interval{0, 3, 6});
+  intervals.emplace_back(Interval{3, 15, 5});
+  intervals.emplace_back(Interval{14, 21, 5});
+  intervals.emplace_back(Interval{23, 28, 5});
+  std::vector<int> expected{4, 3, 7};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundE) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{21, 24, 1});
+  intervals.emplace_back(Interval{0, 5, 1});
+  intervals.emplace_back(Interval{11, 18, 1});
+  intervals.emplace_back(Interval{28, 35, 1});
+  intervals.emplace_back(Interval{5, 11, 1});
+  intervals.emplace_back(Interval{18, 21, 1});
+  intervals.emplace_back(Interval{35, 40, 1});
+  intervals.emplace_back(Interval{24, 28, 1});
+  std::vector<int> expected{1, 4, 2, 5, 0, 7, 3, 6};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundF) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 5, 10});
+  intervals.emplace_back(Interval{5, 11, 100});
+  intervals.emplace_back(Interval{11, 18, 5});
+  intervals.emplace_back(Interval{18, 21, 8});
+  intervals.emplace_back(Interval{21, 24, 2});
+  intervals.emplace_back(Interval{24, 28, 4});
+  intervals.emplace_back(Interval{28, 35, 6});
+  intervals.emplace_back(Interval{35, 40, 7});
+  std::vector<int> expected{0, 1, 2, 3, 4, 5, 6, 7};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}
+
+TEST(IntervalsTest, TestOverlapFoundG) {
+  std::vector<Interval> intervals;
+  intervals.emplace_back(Interval{0, 5, 1});
+  intervals.emplace_back(Interval{0, 5, 1});
+  intervals.emplace_back(Interval{0, 5, 1});
+  intervals.emplace_back(Interval{0, 5, 1});
+  intervals.emplace_back(Interval{0, 5, 1});
+  std::vector<int> expected{0};
+  ASSERT_EQ(IntervalProcessor::filterOverlaps(intervals), expected);
+}