Summary
This RFC proposes DialectInterface for OperationEquvalence and CSE to allow downstream users to ignore specific optional attributes in CSE. Prototype implementation is here: ⚙ D154512 [Not-For-Commit][mlir][CSE][OperationSupport] Introduce DialectInterfaces for CSE
Background
Previous thread: CSE with optional/mergable attributes
In CIRCT there is an optional attribute “sv.namehint” which is meant to be “hint” for Verilog Emitter to produce more readable outputs. Frontends attach namehints to operations but we don’t want to block optimizations because of the existence of namehints. Therefore we want to make CSE ignore these attributes but I think the current CSE is not extensible from the user side. For example we want to CSE these three operations and merge attributes in the specific rule:
%0 = comb.and %a, %b {sv.namehint = "_bad_name1"} : i1
%1 = comb.and %a, %b {sv.namehint = "good_name"} : i1
%2 = comb.and %a, %b {sv.namehint = "_bad_name2"} : i1
return %0, %1, %2
=>
%0 = comb.and %a, %b {sv.namehint = "good_name"} : i1 // we want to prioritize `good_name`.
return %0, %0, %0
Proposed solution
CSE pass basically consists of two parts; OperationEquivalence(hash operation, check operation equivalence) and CSE transform so we want to introduce DialectOperationEquivalenceInterface and DialectCSEInterface to hook these two components respectively.
DialectOperationEquivalenceInterface
This interface controls how attributes participate in the hashing or equivalence checking.
class DialectOperationEquivalenceInterface {
/// Returns true if the given 'attr' contains attributes that can be
/// ignored in the equivalence comparison.
virtual bool containsNonEssentialAttribute(DictionaryAttr attr) const {
return false;
}
/// Returns true if the given named attribute 'namedAttr' can be ignored
/// in the equivalence comparison.
virtual bool isNonEssentialAttribute(NamedAttribute namedAttr) const {
return false;
}
};
Hashes of attributes which isNonEssentialAttribute returns true will not be combined into an operation hash. In CIRCT, the implementation would be:
bool containsNonEssentialAttribute(DictionaryAttr attr) { return attr.contains("sv.namehint");}
bool isNonEssentialAttribute(NamedAttribute namedAttr) { return namedAttr.getName() == "sv.namehint";}`
DialectCSEInterface
This interface controls how operation attributes are merged into one to another while CSE.
class DialectCSEInterface {
/// Returns a dictionary attribute that are combined derived from
/// `opAttr` (attributes attached to an op that is going to be eliminated) and
/// `existingOpAttr`.
virtual DictionaryAttr
combineDiscardableAttributes(DictionaryAttr opAttr,
DictionaryAttr existingOpAttr) const {
return existingOpAttr;
}
};
The CIRCT implementation would be like this:
DictionaryAttr combineDiscardableAttributes(DictionaryAttr opAttr,
DictionaryAttr existingOpAttr) const override {
if (auto src = opAttr.getAs<StringAttr>("sv.namehint")) {
// If both has "sv.namehint" attr, set the smaller value.
if (auto dest = existingOpAttr.getAs<StringAttr>("sv.namehint")) {
...
// If `src` is better name than `dest`, then update and return attributes.
}
}
return existingOpAttr;
}
Alternative considered
- Operational Equivalence already has a flag to ignore locations on operations or block arguments so it might make sense to extend the flag to ignore discardable (or dialect) attributes. I prefer the proposed dialect interface approach as I believe the approach is more extensible.
- Another possible solution might be to expose CSE as a library which parametrizes SimpleOperationInfo.
Feedback is highly appreciated!
