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

MLIR Mapping #51

Merged
merged 5 commits into from
Nov 17, 2023
Merged

MLIR Mapping #51

merged 5 commits into from
Nov 17, 2023

Conversation

Tikipiou
Copy link
Collaborator

@Tikipiou Tikipiou commented Nov 9, 2023

Description

This pull request introduces the first version of the tools for mapping MLIR components to Graph components, as introduced in #40.

The primary components included in this pull request are:

  • MLIRMapper.cpp: This class maps MLIR components to Graph components.
  • GraphParser.cpp: This updates the parser to handle .mlir files, converting them into MLIR components and then invoking the MLIR mapper.

What’s new

  • MLIRMapper class

Testing

  • Verified the functionality and accuracy of parsed data by testing with sample .mlir files.

@Tikipiou Tikipiou requested a review from lucas-rami November 9, 2023 15:30
Copy link
Member

@lucas-rami lucas-rami left a comment

Choose a reason for hiding this comment

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

I made a couple changes myself for the sake of time (and considering that I took long to check out the code, sorry!). Big ones detailed below:

  • I replaced the new port-index-fixing magic with the updated one from DOTPrinter.cpp.
  • I removed the new's for the graph nodes and edges, which were not necessary since the Graph stores raw nodes and edges as opposed to pointers to them i.e. it "owns" its data. Besides, since they were never deleted they were causing memory leaks.

There are a couple things in this code we will for sure go back to and change, but I merge this as is nonetheless since

  1. these are things that are prompted by recent changed on main which I didn't tell you about for simplicity (e.g., we now have a much much better way of naming operations across passes in the form of dynamatic::NamingAnalysis, which makes the current "count-per-operation-type naming system" unnecessary), and
  2. I'm not yet completely sure of how this is going to tie in to the rest of the visualizer so I don't want to make you iterate on code I'm not sure will be useful/relevant in the near future. This is a good base to start on, so it will do!

@lucas-rami lucas-rami merged commit fae090a into main Nov 17, 2023
@lucas-rami lucas-rami deleted the visual-dataflow-mlir branch July 23, 2024 17:11
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.

2 participants