[llvm-branch-commits] [flang] [mlir] [mlir][Transforms] Support 1:N mappings in `ConversionValueMapping` (PR #116524)
Matthias Springer via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Dec 22 05:26:57 PST 2024
================
@@ -63,80 +113,140 @@ static OpBuilder::InsertPoint computeInsertPoint(Value value) {
return OpBuilder::InsertPoint(insertBlock, insertPt);
}
+/// Helper function that computes an insertion point where the given values are
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(ArrayRef<Value> vals) {
+ assert(!vals.empty() && "expected at least one value");
+ OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
+ for (Value v : vals.drop_front())
+ pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+ return pt;
+}
+
//===----------------------------------------------------------------------===//
// ConversionValueMapping
//===----------------------------------------------------------------------===//
+/// A vector of SSA values, optimized for the most common case of a single
+/// value.
+using ValueVector = SmallVector<Value, 1>;
+
namespace {
+
+/// Helper class to make it possible to use `ValueVector` as a key in DenseMap.
+struct ValueVectorMapInfo {
+ static ValueVector getEmptyKey() { return ValueVector{}; }
+ static ValueVector getTombstoneKey() { return ValueVector{}; }
+ static ::llvm::hash_code getHashValue(ValueVector val) {
+ return ::llvm::hash_combine_range(val.begin(), val.end());
+ }
+ static bool isEqual(ValueVector LHS, ValueVector RHS) { return LHS == RHS; }
+};
+
/// This class wraps a IRMapping to provide recursive lookup
/// functionality, i.e. we will traverse if the mapped value also has a mapping.
struct ConversionValueMapping {
/// Return "true" if an SSA value is mapped to the given value. May return
/// false positives.
bool isMappedTo(Value value) const { return mappedTo.contains(value); }
- /// Lookup the most recently mapped value with the desired type in the
+ /// Lookup the most recently mapped values with the desired types in the
/// mapping.
///
/// Special cases:
- /// - If the desired type is "null", simply return the most recently mapped
- /// value.
- /// - If there is no mapping to the desired type, also return the most
- /// recently mapped value.
- /// - If there is no mapping for the given value at all, return the given
- /// value.
- Value lookupOrDefault(Value from, Type desiredType = nullptr) const;
-
- /// Lookup a mapped value within the map, or return null if a mapping does not
- /// exist. If a mapping exists, this follows the same behavior of
- /// `lookupOrDefault`.
- Value lookupOrNull(Value from, Type desiredType = nullptr) const;
+ /// - If the desired type range is empty, simply return the most recently
+ /// mapped values.
+ /// - If there is no mapping to the desired types, also return the most
+ /// recently mapped values.
+ /// - If there is no mapping for the given values at all, return the given
+ /// values.
+ ValueVector lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes = {}) const;
+
+ /// Lookup the given values within the map, or return an empty vector if the
+ /// values are not mapped. If they are mapped, this follows the same behavior
+ /// as `lookupOrDefault`.
+ ValueVector lookupOrNull(const ValueVector &from,
+ TypeRange desiredTypes = {}) const;
/// Map a value to the one provided.
- void map(Value oldVal, Value newVal) {
+ void map(const ValueVector &oldVal, const ValueVector &newVal) {
LLVM_DEBUG({
- for (Value it = newVal; it; it = mapping.lookupOrNull(it))
- assert(it != oldVal && "inserting cyclic mapping");
+ ValueVector next = newVal;
+ while (true) {
+ assert(next != oldVal && "inserting cyclic mapping");
+ auto it = mapping.find(next);
+ if (it == mapping.end())
+ break;
+ next = it->second;
+ }
});
- mapping.map(oldVal, newVal);
- mappedTo.insert(newVal);
+ mapping[oldVal] = newVal;
+ for (Value v : newVal)
+ mappedTo.insert(v);
}
- /// Drop the last mapping for the given value.
- void erase(Value value) { mapping.erase(value); }
+ /// Drop the last mapping for the given values.
+ void erase(ValueVector value) { mapping.erase(value); }
private:
/// Current value mappings.
- IRMapping mapping;
+ DenseMap<ValueVector, ValueVector, ValueVectorMapInfo> mapping;
/// All SSA values that are mapped to. May contain false positives.
DenseSet<Value> mappedTo;
};
} // namespace
-Value ConversionValueMapping::lookupOrDefault(Value from,
- Type desiredType) const {
- // Try to find the deepest value that has the desired type. If there is no
- // such value, simply return the deepest value.
- Value desiredValue;
+ValueVector
+ConversionValueMapping::lookupOrDefault(ValueVector from,
+ TypeRange desiredTypes) const {
+ // Try to find the deepest values that have the desired types. If there is no
+ // such mapping, simply return the deepest values.
+ ValueVector desiredValue;
do {
- if (!desiredType || from.getType() == desiredType)
+ // Store the current value if the types match.
+ if (desiredTypes.empty() || TypeRange(from) == desiredTypes)
desiredValue = from;
- Value mappedValue = mapping.lookupOrNull(from);
- if (!mappedValue)
+ // If possible, Replace each value with (one or multiple) mapped values.
+ ValueVector next;
----------------
matthias-springer wrote:
I struggled with this function quite a lot. There are few details that are tricky.
> To me this means we have no longer a distinct way to get the "most-recently" mapped value.
That's correct.
> That said, in practice the latter case may only succeed if there was a N:M materialization right?
Correct.
> Is my impression correct that the second case can therefore just be seen as an optimization to reuse a materialization rather than creating a new one (for when the desired types match).
Yes. If we miss a materialization, it's not a problem for a functional point of view. (We will just create another one.) But missing an actual replacement could be a problem. That's why the implementation just looks up values individually. That way it is guaranteed that we find all replacements.
> Regarding the first case: What confuses me a bit is the append range behaviour. Playing with asserts I found that in the test suite it->second and from are never larger than 1 at the same time. Can this happen in theory?
Could I e.g. have one pattern replacing a tuple<i64, i64> with two i64s and those replaced by another pattern with two i32s? And any subsequent operation (or target materialization for the operation) using the tuple<i64, i64> would receive the four i32s?
Yes, that's what I had in mind. It looks like we have no test case that exercises this...
There was another design that I played with: having two separate mapping for replacements and materializations:
```c++
DenseMap<Value, ValueRange> replacements;
// `materializations` can store multiple materializations to different types.
DenseMap<SmallVector<Value>, SmallVector<ValueRange>> materializations;
```
My initial lookup mechanism worked as follows:
1. Find the most recently mapped values in `replacements`, regardless of the type. Let's call them `repl`.
2. Try to find a materialization to the correct types.
This approach avoids the problem of multiple possible lookup paths. But there is a problem: let's say a pattern P1 without a type converter replaces SSA value A with B, but A and B have the same type. Then another pattern P2 with a type converter (and a registered materialization callback) replaces B with C, and B and C have different types. If we were to lookup the most recently mapped value of A immediately, the conversion driver would try to build a materialization from type(A) to type(C) with P1's type converter (which will fail).
After a few more experiments, I decided to stay as close as possible to the current `ConversionValueMapping` implementation. In particular, not splitting the `IRMapping` into multiple things.
https://github.com/llvm/llvm-project/pull/116524
More information about the llvm-branch-commits
mailing list