$_self will be the base type of the entity that attaches to a predicate

After D110199, in DRR, the matching of type/attribute will turn into a function call. To reduce the complexity of the implementation, the type of $_self is the base type, i.e., Value or Attribute. I notice that some attribute constraint may suppose it’s the derived type. For example,

def TrueBoolAttr : AttrConstraint<CPred<"$_self.getValue()">>;

It supposes that $_self will be BoolAttr. In fact it’s implementation defined. The ideal way is to have it in the form

def TrueBoolAttr : AttrConstraint<CPred<"$_self.cast<BoolAttr>().getValue()">>;

That CL is still under review so folks have some time to clean this up and any suggestion for this change is welcomed.

– Chia-hung

Isn’t this turning something that is a static check into a runtime check?
I mean that def TrueBoolAttr : AttrConstraint<CPred<"$_self.getValue()">>; would fail to build if used in a place where $_self isn’t a BoolAttr (or does not expose a boolean getValue()), while $_self.cast<BoolAttr>().getValue() will only fail at runtime (and only with assertions enabled) in the cast itself.

Yes, it is. But it depends on the codegen of DRR. We have the code like

auto tblgen_attr = op0->getAttrOfType<::mlir::BoolAttr>("attr1");(void)tblgen_attr;
if (!(tblgen_attr)){
if (!tblgen_attr.getValue()) {

If tblgen_attr is an Attribute type or if we verify the additional constraint first, then they may not be able to assume it’s a BoolAttr in this case.

I think we don’t say it’ll be the derived type in OpBase.td and so far, I see most cases are supposing it’s the base type. Thus I guess the impact of switching from static check to runtime check may not have a big impact, what do you think?

But that’s not possible with the get there is it? It is requesting the derived attribute type and would return null if not of that type.

True, but we aren’t doing that today so why would we change that? It seems like verifying the derived attribute type is cheap to do and provides all following verification more structure.

Yes so we may need to update the documentation either way and it could be due to the documentation that folks haven’t been assuming. But seems like some use this already. What would the cost be to making this the expected type (derived) vs base Attribute? (Infra side and user side)

If it is the derived type then we still be able to capture it with the base type. The purpose here will become pure isa<> testing. (Of course we shouldn’t do it like that)

I think the cost is existing when the assertion is enabled. But yes, I’m not sure if it gives a big impact.

I’m thinking a case that if an Attribute has been subclass many times and they shared the same predicate, then I need to use the type of the least common ancestor as the parameter type. In this case, I may use template parameter to solve this problem.

The cost for infra side will be the support of template. If we choose the use of base type, than the cost for user will be the assertion in cast<> if assertion is enabled.

Template one is not hard and I just thought the use of base type will be easier, but it seems the template one is ideal, right? :slight_smile: