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.

Sign up for GitHub

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

Jump to bottom

Implement graphviz #23

Merged
merged 11 commits into from
Aug 22, 2023
Merged

Implement graphviz #23

merged 11 commits into from
Aug 22, 2023

Conversation

k0aki
Copy link
Contributor

@k0aki k0aki commented Aug 11, 2023

Implement graphviz for sonatina IR with dot2 crate.

Y-Nak
Y-Nak reviewed Aug 12, 2023
View reviewed changes
Copy link
Collaborator

@Y-Nak Y-Nak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Cool! You are on the right track. I like Display* thin wrapper types.
I left some comments, I think we could merge this PR soon.

crates/ir/Cargo.toml Outdated Show resolved Hide resolved
crates/ir/src/graphviz/function.rs Outdated Show resolved Hide resolved
crates/ir/src/graphviz/block.rs Outdated Show resolved Hide resolved
crates/ir/src/graphviz/block.rs Outdated Show resolved Hide resolved
crates/ir/src/graphviz/block.rs Outdated Show resolved Hide resolved
crates/ir/src/insn.rs Outdated Show resolved Hide resolved
crates/ir/src/insn.rs Outdated Show resolved Hide resolved
crates/ir/src/insn.rs Outdated Show resolved Hide resolved
crates/ir/src/insn.rs Outdated Show resolved Hide resolved
crates/ir/src/module.rs Outdated Show resolved Hide resolved
@k0aki @Y-Nak
Remove unused dep
ce286df 
Co-authored-by: Yoshitomo Nakanishi <[email protected]>
@k0aki k0aki force-pushed the graphviz branch 2 times, most recently from db365b5 to b08c2f6 Compare August 14, 2023 08:00
@k0aki @Y-Nak
Fix instruction string representation
327e706 
Co-authored-by: Yoshitomo Nakanishi <[email protected]>
Y-Nak
Y-Nak reviewed Aug 15, 2023
View reviewed changes
Copy link
Collaborator

@Y-Nak Y-Nak left a comment

Choose a reason for hiding this comment

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

Ok, thanks! Almost perfect, I'll merge the PR after the one nit left is addressed.

crates/ir/src/graphviz/block.rs Outdated Show resolved Hide resolved
@k0aki @Y-Nak
Fix commit 9a80406
e811a63 
Co-authored-by: Yoshitomo Nakanishi <[email protected]>
Y-Nak
Y-Nak approved these changes Aug 22, 2023
View reviewed changes
Copy link
Collaborator

@Y-Nak Y-Nak left a comment

Choose a reason for hiding this comment

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

Cool!

@Y-Nak Y-Nak merged commit d316edc into fe-lang:main Aug 22, 2023
8 of 9 checks passed
@k0aki k0aki deleted the graphviz branch September 12, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Y-Nak Y-Nak Y-Nak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k0aki @Y-Nak