Skip to content

Conversation

@mehrdad2m
Copy link
Collaborator

This PR migrates several new StableHLO dialect operations/attributes/types to the xdsl_jax project that was previously implemented in PennyLane repository.

List of changes:

  1. Categorizes the new operations based on StableHLO specification document here
  2. Previously implemented operations are still intact and renamed to _stablehlo_upstream.py. These operations need to be categorized gradually as well.
  3. Adds necessary attributes and types as per the StableHLO specification, plus some general traits and constraints that can be upstreamed to xdsl in the xdsl-extras module.
  4. It adds pytest tests to test general format parse/print roundtrip tests for the new operations. (Note that this creates a disparity in testing as the current operations are tested using lit tests while the new operations are using pytest. We should ideally unify the testing infrastructure in future.

for reference, here are the PRs that originally implemented the new operations:
PennyLaneAI/pennylane#8036
PennyLaneAI/pennylane#8084
PennyLaneAI/pennylane#8113
PennyLaneAI/pennylane#8217
PennyLaneAI/pennylane#8281
PennyLaneAI/pennylane#8310

This commit migrates several new StableHLO dialect operations to the xdsl_jax project.
Additionally, it includes necessary attributes and types as per the StableHLO specification, plus some general traits and constraints that can be upstreamed to xdsl in the xdsl-extras module
Moreover, It adds pytest tests to test general format parse/print roundtrip tests for the new operations.
@mehrdad2m mehrdad2m requested a review from mudit2812 October 23, 2025 13:39
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious to hear what the xDSL team thinks about these fixtures that we use to enable lit testing with pytest. I think eventually we should migrate the lit tests to just use regular filecheck, the same way that the other dialects are tested in xDSL (example), and add pytest tests that are specifically for unit testing (example).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I would go for filecheck instead of this TBH, does the stablehlo repo have filecheck tests that we could reuse?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree. I just wanted to have this PR to start discussing about the necessary changes before implementing anything new. I think we originally opted for the fixtures since Pennylane does not use lit test at all. For the simple roundtrip testing, using regular lit tests are definitely easier.

Copy link
Collaborator

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mehrdad2m do you think it might make more sense to break this up into smaller PRs the same way you did in PennyLane? I ask because the large size makes the PR harder to review, and also because I think that there should be changes and additions to the testing before we merge everything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not replace it ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah ideally this file should be deleted and its operations would be dispatched to other files. However, that can take a little more time cause we should make sure these ops are also up to date with the new added traits/type system.

@superlopuh
Copy link
Member

This is fantastic, I would propose converting this PR to draft, and to take a subset of the changes to review those specifically. It feels like starting with the attributes might be a good idea to scope things a little bit, and let us iterate on style and other things outside of functionality.

@mehrdad2m
Copy link
Collaborator Author

@mehrdad2m do you think it might make more sense to break this up into smaller PRs the same way you did in PennyLane? I ask because the large size makes the PR harder to review, and also because I think that there should be changes and additions to the testing before we merge everything.

Yeah we can definitely do that. This PR is essentially a copy of the PennyLane version of the dialect, hence the size. I wanted to make sure that everything works before adding any new testing or changing any of the infrastructure. We can for sure add things more gradually.

@mehrdad2m
Copy link
Collaborator Author

This is fantastic, I would propose converting this PR to draft, and to take a subset of the changes to review those specifically. It feels like starting with the attributes might be a good idea to scope things a little bit, and let us iterate on style and other things outside of functionality.

sounds good to me. We'll make smaller PRs to add things more gradually.

@mehrdad2m mehrdad2m marked this pull request as draft October 23, 2025 15:15
superlopuh added a commit that referenced this pull request Nov 4, 2025
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

Successfully merging this pull request may close these issues.

4 participants