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

F frontend #1275

Merged
merged 16 commits into from Jun 27, 2023
Merged

F frontend #1275

merged 16 commits into from Jun 27, 2023

Conversation

acalotoiu
Copy link
Contributor

@acalotoiu acalotoiu commented Jun 11, 2023

Currently, the core Fortran frontend is ready for comment.
Tests have also been added
There are many features and improvements possible, however, I want to focus on merging the current working prototype as soon as it is DaCe compliant to keep the size of the PR to a minimum as it is already very large.

Copy link
Collaborator

@phschaad phschaad left a comment

Choose a reason for hiding this comment

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

Some preliminary comments

dace/sdfg/utils.py Show resolved Hide resolved
dace/frontend/fortran/ast_transforms.py Outdated Show resolved Hide resolved
dace/frontend/fortran/ast_transforms.py Outdated Show resolved Hide resolved
dace/frontend/fortran/ast_utils.py Show resolved Hide resolved
outpath = args.out

# Load SDFG
sdfg = fortran_parser.create_sdfg_from_fortran_file(filepath)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A bit sad that fcdc has to live separate from sdfgcc, since they seem functionally and programatically identical in all but one (or two, if you count the import) line.

What do you think about just using sdfgcc and detecting whether something is an SDFG or Fortran file? The name sdfgcc may be sub-optimal in that case, maybe it makes sense to have a dace-cc that autodetects (or is told explicitly what it is with --source-lang, and have sdfgcc alias dace-cc --source-lang=sdfg and fcdc alias dace-cc --source-lang=fortran.

Not a crucial point, but the code duplication is a bit sad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed - this was done by request of @tbennun . What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about this. We can discuss

Copy link
Contributor

Choose a reason for hiding this comment

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

Seconding the idea of dace-cc with autodetection and additional parameters.

tests/fortran/view_test.py Show resolved Hide resolved
dace/frontend/fortran/ast_components.py Outdated Show resolved Hide resolved
dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
outpath = args.out

# Load SDFG
sdfg = fortran_parser.create_sdfg_from_fortran_file(filepath)
Copy link
Contributor

Choose a reason for hiding this comment

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

Seconding the idea of dace-cc with autodetection and additional parameters.

dace/frontend/fortran/ast_components.py Show resolved Hide resolved
dace/frontend/fortran/ast_transforms.py Outdated Show resolved Hide resolved
dace/frontend/fortran/ast_utils.py Outdated Show resolved Hide resolved
dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@phschaad phschaad left a comment

Choose a reason for hiding this comment

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

LGTM, pls fix left over debug prints and commented out code

dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
dace/frontend/fortran/fortran_parser.py Outdated Show resolved Hide resolved
@acalotoiu acalotoiu merged commit dcc284d into master Jun 27, 2023
9 checks passed
@tbennun tbennun deleted the f_frontend branch June 27, 2023 11:01
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.

None yet

5 participants