Basically, I want a version of DefaultValuedAttr which doesn’t store attribute if it equal to default value.
Specifically, I want for tblgen to generate smth like this for op build methods
if (fmf != <default value>)
odsState.addAttribute("fmf", ::mlir::LLVM::FMFAttr::get(fmf, odsBuilder.getContext()));
Generated accessors can already handle this case (treat missing attribute as default value).
Does it make sense?
Any ideas how it should like from API side (because we probably want to preserve old behavior for existing uses of DefaultValuedAttr).
Also, it is not the same as OptionalAttr<DefaultValuedAttr<...>> because in this case missing attribute will be treated as separate state, complicating op API.
I was actually planning to go the opposite way and moving the default attribute construction to the builder from the get function to ensure the in memory form is always the same, avoid construction in the get method and reduce inconsistencies.
Now excluding it from printing the default in the pretty form sounds fine.
Note that in External function declaration has changed. Help needed, the consensus was to go in a different direction for sym_visibility attribute (not have the default values attribute be present explicitly at all). The use case these might be slightly different here because the access to this attribute will be hidden behind SymbolOpInterface though. So if we generically look at all attributes of a Symbol we may not see sym_visibility.
Yes which is something I’m not a big fan of: e.g., if the in memory format is not complete and one is required to use an interface to query information about an operation. We have that today already, getAttr("X") and XAttr() may return different results for what is conceptually supposed to be the same query. I am to blame for that, not sure it pays for itself/inconsistency is worth it (dropping it when printing, adding it while parsing seems fine but inconsistent answers when querying during compile time using conceptually equivalent methods feels off).
Given my experience with bindings, I second @jpienaar opinion here: confining part of the op semantics to the logic of a C++ getter function makes it hard to reason about the op in a generic way (or in absence of C++).
For me, it feels that the ambiguity starts at the op semantics level. Taking the visibility example, both the absence of sym_visibility attribute andsym_visibility = private have the same effect. Having two ways of indicating the same property is what leads to further complications. Jacques’ way to fix this is to say sym_visibility is always present. Another possibility is to say that sym_visibility = private should not exist, and we always represent the equivalent semantics by not having the attribute.
If we go for using a default modeled as missing attribute, I would hope to see support for this in ODS specifications. Then, at the C++ layer, the getter would still return private but we have a consistent way to express it.
The only advantages of default over stating it explicitly that I can see are less verbosity in printing and smaller in-memory size of IR. In this particular case, as it is per symbol definition only, I am not concerned about the latter. And as this is an IR and not a programming language, I would prefer it to be explicit, because that is one less “implicit contract” to know while reading and producing IR.