core: with-type-constraint

[alexarice]

Experiment in using Constraint variables to grab a type from an attribute, and an example of using this for arith.constant.

Unsure if this a good idea or cooking too hard.

Edit: I also couldn't really figure out how TypedAttribute worked, and was not sure if the functionality could be reused (I have use cases in mind where the associated type would not be a parameter)

[alexarice]

I also believe both failing tests are actually bugs exposed by this change rather than bugs with this change

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.20%. Comparing base (1fab125) to head (52e7756).

Additional details and impacted files

@@                      Coverage Diff                       @@
##           alexarice/constraints-v1.5    #3318      +/-   ##
==============================================================
- Coverage                       90.21%   90.20%   -0.01%     
==============================================================
  Files                             459      459              
  Lines                           57655    57654       -1     
  Branches                         5561     5561              
==============================================================
- Hits                            52011    52008       -3     
+ Misses                           4202     4201       -1     
- Partials                         1442     1445       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[superlopuh]

On one hand, you're cooking pretty hard indeed, on the other this is great. Probably worth integrating with TypedAttribute[Attribute]

[math-fehr]

Do you think you could have an inheritance like WithType -> TypedAttribute?
WithType just means your attribute has a canonical type. On the other hand, TypedAttribute means that you have a type, that the type is stored in one of the parameters, and that the type is parsed after the :. So TypedAttribute should be a strict subset of WithType.

My goal with TypedAttribute was originally to handle all cases that are (or can be) parsed with :.
You said you had in mind attributes that do not have types in their parameters, what example do you have in mind?

[alexarice]

Not sure if it will show up as the repository is private but see here for an example:

https://github.com/xdslproject/inconspiquous/blob/0f6ab2733722ba276408716173fb0a25c467f377/inconspiquous/dialects/qubit.py#L42

I should note that this is really the only actual use case I have, and it could be done with a custom constraint (and needs the corresponding range constraint anyway), as long as the change to verifying property/attribute constraints during inference in the PR is merged

[alexarice]

@math-fehr with the TypedAttribute class, is it guaranteed that there is a type attribute? Pyright doesn't seem to know this but I found a place where I think it was being runtime enforced

[math-fehr]

From what I recall, type is not expected to exist. Instead there was get_type_index which would tell you what is the parameter index that contains the type.
So if you have the parameters a and b, where a is the type, then get_type_index would return 0.

Note that I wrote this 2-3 years ago, and I do think this design sucks, so I'm absolutely fine if you want to change some parts of it.

[alexarice]

This is the code I was referring to:

xdsl/xdsl/irdl/attributes.py

Lines 163 to 181 in 61bf632

	if issubclass(pyrdl_def, TypedAttribute):
	pyrdl_def = cast(type[TypedAttribute[Attribute]], pyrdl_def)
	try:
	param_names = [name for name, _ in param_hints]
	type_index = param_names.index("type")
	except ValueError:
	raise PyRDLAttrDefinitionError(
	f"TypedAttribute {pyrdl_def.__name__} should have a 'type' parameter."
	)
	typed_hint = param_hints[type_index][1]
	if get_origin(typed_hint) is Annotated:
	typed_hint = get_args(typed_hint)[0]
	type_var = get_type_var_mapping(pyrdl_def)[1][AttributeCovT]
	if typed_hint != type_var:
	raise ValueError(
	"A TypedAttribute `type` parameter must be of the same type"
	" as the type variable in the TypedAttribute base class."
	)

perhaps this is just checking that the type parameter can be found in the correct place.

I'll rebase this in the meantime

[alexarice]

I looked again and IntegerAttr seems to be the only TypedAttribute in xdsl

[alexarice]

Would you expect DenseIntOrFPElementsAttr to be a TypedAttribute? I'm beginning to think that the functionality of "having an associated type" and "printing and parsing with/without a type" are slightly separate

[alexarice]

I want to add that I've been experimenting with traits like this a bit more, and it's all kind of evil as it all depends on what order you "verify" constraints in. Ideally, I feel that the "verify" methods should not solve any constraints and all solving of constraint variables should be done by a separate system which does not depend on the order of constraints, though this would need a bigger refactor.

If anyone has had any ideas on this in general, it would be good to know.

[math-fehr]

Would you expect DenseIntOrFPElementsAttr to be a TypedAttribute? I'm beginning to think that the functionality of "having an associated type" and "printing and parsing with/without a type" are slightly separate

Sorry I thought I answered this but I forgot to.
If I recall, these are merged in MLIR. In MLIR, all attribute have a type field (most of them are just None though).
So if you have an associated type, it will be printed/parsed with the colon. That's my recollection at least.
We are free to do however we want, though I would likely say that these should be merged.

[math-fehr]

I want to add that I've been experimenting with traits like this a bit more, and it's all kind of evil as it all depends on what order you "verify" constraints in. Ideally, I feel that the "verify" methods should not solve any constraints and all solving of constraint variables should be done by a separate system which does not depend on the order of constraints, though this would need a bigger refactor.

If anyone has had any ideas on this in general, it would be good to know.

I think we need to provide an actual order on the traits yes.
This is something that is also broken in MLIR btw, and I am not completely sure on how to fix this in general

[alexarice]

As a little update, I had a small discussion with @PapyChacal about how to update the constraint system to avoid these ordering problems and be a bit more xdsl-like (+ hopefully slightly simpler and more performant), but will probably only be able to work on that in a week's time.

The constraints in this PR are probably still interesting (at least to me), even if the mechanism for solving them is slightly messy atm. Would we still want to merge this if I could think about how to merge WithType and TypedAttribute?

[superlopuh]

Temporary solutions have a tendency to become permanent. A week is not a long time, and I would feel better if we could all be happy with this PR before merging it. If you see a better to implement it in the relatively near future, then let's wait a little bit longer.

[alexarice]

Rebased onto #3456. I can't remember exactly what we wanted in terms of WithType/TypedAttribute