Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cir.vec.create syntax is a bit confusing #541

Open
bcardosolopes opened this issue Apr 10, 2024 · 3 comments
Open

cir.vec.create syntax is a bit confusing #541

bcardosolopes opened this issue Apr 10, 2024 · 3 comments
Assignees

Comments

@bcardosolopes
Copy link
Member

Take this example:

%1 = ...
%2 = cir.vec.create(%1, %1 : !s32i, !s32i) : !cir.vector<!s32i x 2>

This is building a vector with two elements [%1, %1], right? Assuming I'm right, this is a bit confusing to read for two reasons:

  • Only the second %1 has the type
  • Why we need the third operand just for the type?

It's possible we add constraints as to only print %2 = cir.vec.create(%1, %1) : !cir.vector<!s32i x 2>, since !cir.vector's elt type has all the info for logic to figure out. Alternatively, we could only add the type for the first operand, since all the others have to be the same: %2 = cir.vec.create(%1 : !s32i, %1) : !cir.vector<!s32i x 2>.

Thoughts?

cc: @gitoleg @dkolsen-pgi

@dkolsen-pgi
Copy link
Collaborator

Those are valid criticisms of the cir.vec.create syntax. I fully agree that the syntax is overly redundant. That was the first new operation that I added to ClangIR, so my skills at constraints and assembly format were still developing. The syntax (%1, %1 : !s32i, !s32i) is just the assembly format

`(` ($elements^ `:` type($elements))? `)`

which was the simplest way I could find to get it to work.

I like the suggestion of cir.vec.create(%1, %1) : !cir.vector<!s32i x 2>, leaving out the types of the element values since they can be inferred from the type of the vector. I will try to make this change, to help with my learning of constraints on operations.

@dkolsen-pgi dkolsen-pgi self-assigned this Apr 10, 2024
@dkolsen-pgi
Copy link
Collaborator

Now I remember what the complication was. All of the element values must be of the same type. But I couldn't find an example of another operation like that. I know how to add a constraint to the operation definition that requires the type of an input to match the element type of a vector. But I don't know how to specify that all the items in a variadic list of inputs need to match the element type of a vector.

Is there an MLIR operation that has similar constraints that I can use as an example? If not, do you have any pointers about how to go about this?

@bcardosolopes
Copy link
Member Author

@dkolsen-pgi thanks for looking at this. By the way, I think you did a great job introducing this, I couldn't notice the output during code review and the approach is totally fine, just a note that we can probably make it a bit better (feel free to leave this a "good-first-issue" to a newcomer if you prefer). I learned about this while looking at inline asm in #512

Is there an MLIR operation that has similar constraints that I can use as an example?

The most similar ones seems to be SameOperandsShape and SameOperandsElementType from OpBase.td, but not sure how well they apply - sometimes these are more tricky than it seems.

If not, do you have any pointers about how to go about this?

I started introducing a few CIR specific ones, see SameFirstOperandAndResultType, SameSecondOperandAndResultType and SameFirstSecondOperandAndResultType. Probably something along these lines should work too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants