Skip to content

ISLE: split algebraic.isle into several files#6140

Merged
jameysharp merged 4 commits intobytecodealliance:mainfrom
Kmeakin:split-isle-files
Apr 11, 2023
Merged

ISLE: split algebraic.isle into several files#6140
jameysharp merged 4 commits intobytecodealliance:mainfrom
Kmeakin:split-isle-files

Conversation

@Kmeakin
Copy link
Contributor

@Kmeakin Kmeakin commented Apr 4, 2023

Split algebraic.isle into several files, grouping similar operators together.

@Kmeakin Kmeakin requested a review from a team as a code owner April 4, 2023 01:51
@Kmeakin Kmeakin requested review from elliottt and removed request for a team April 4, 2023 01:51
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Apr 4, 2023
@Kmeakin Kmeakin force-pushed the split-isle-files branch from 4e9ffa7 to 612f923 Compare April 4, 2023 02:23
@jameysharp
Copy link
Contributor

jameysharp commented Apr 4, 2023

It's definitely a good idea to split up these rules, and I like the division you've chosen between operations. But I think you probably intended to delete cranelift/filetests/filetests/egraph/algebraic.clif too, right?

@jameysharp jameysharp mentioned this pull request Apr 4, 2023
Merged
@Kmeakin
Copy link
Contributor Author

Kmeakin commented Apr 5, 2023

It's definitely a good idea to split up these rules, and I like the division you've chosen between operations. But I think you probably intended to delete cranelift/filetests/filetests/egraph/algebraic.clif too, right?

Yes, that should have been removed

@elliottt elliottt removed their request for review April 5, 2023 15:38
@Kmeakin Kmeakin force-pushed the split-isle-files branch 2 times, most recently from 515e67e to a29c7a4 Compare April 6, 2023 23:02
jameysharp
jameysharp reviewed Apr 7, 2023
View reviewed changes
Copy link
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

I've confirmed that everything here is just the rearrangement described in the commit message, so thank you for that!

I see that algebraic.clif is still not deleted though, and I've made one other request below. Could you take care of those? I'd be happy to merge this on Monday.

@Kmeakin
Copy link
Contributor Author

Kmeakin commented Apr 7, 2023

I see that algebraic.clif is still not deleted though, and I've made one other request below. Could you take care of those? I'd be happy to merge this on Monday.

I have deleted algebraic.clif several times now, very confused

@Kmeakin Kmeakin requested review from cfallin and jameysharp April 10, 2023 15:56
@afonso360 afonso360 mentioned this pull request Apr 11, 2023
Merged
jameysharp
jameysharp approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good!

I've deleted algebraic.clif again for you. I think your rebase steps aren't going quite right, somehow.

@jameysharp jameysharp enabled auto-merge April 11, 2023 21:26
@jameysharp jameysharp added this pull request to the merge queue Apr 11, 2023
Merged via the queue into bytecodealliance:main with commit b9a5814 Apr 11, 2023
brendandburns pushed a commit to brendandburns/wasmtime that referenced this pull request Apr 13, 2023
@Kmeakin @jameysharp @brendandburns
ISLE: split algebraic.isle into several files (bytecodealliance#6140)
d63621d 
* ISLE: split algebraic.isle into several files

* delete `algebraic.clif`

* Add `README.md`

* Remove old `algebraic.clif` tests

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Apr 16, 2023
@Kmeakin @jameysharp @eduardomourar
ISLE: split algebraic.isle into several files (bytecodealliance#6140)
5f4362f 
* ISLE: split algebraic.isle into several files

* delete `algebraic.clif`

* Add `README.md`

* Remove old `algebraic.clif` tests

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
@Kmeakin Kmeakin deleted the split-isle-files branch January 30, 2024 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cfallin cfallin Awaiting requested review from cfallin

1 more reviewer

@jameysharp jameysharp jameysharp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cranelift Issues related to the Cranelift code generator

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Kmeakin @jameysharp @cfallin