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

--dump-all-passes-fortran seems to be calling gfortran somehow #2744

Closed
certik opened this issue Oct 27, 2023 · 24 comments · Fixed by #2825
Closed

--dump-all-passes-fortran seems to be calling gfortran somehow #2744

certik opened this issue Oct 27, 2023 · 24 comments · Fixed by #2825

Comments

@certik
Copy link
Contributor

certik commented Oct 27, 2023

So I get the following errors:

$ lfortran examples/expr2.f90 --dump-all-passes-fortran 
expr2.out.tmp.o.tmp.f90:3:16:

    3 | integer(4) :: __1_t
      |                1
Error: Invalid character in name at (1)
expr2.out.tmp.o.tmp.f90:4:16:

    4 | integer(4) :: __1_u
      |                1
Error: Invalid character in name at (1)
expr2.out.tmp.o.tmp.f90:5:16:

    5 | integer(4) :: __1_v
      |                1
Error: Invalid character in name at (1)

Rather, I would expect that --dump-all-passes-fortran just dumps the ASR passes as Fortran code, but otherwise does whatever lfortran would do without this option, which in the case above would just compile and run the code:

$ lfortran examples/expr2.f90                           
$

As a workaround, one can do:

$ lfortran examples/expr2.f90 --dump-all-passes-fortran --show-llvm
@Thirumalai-Shaktivel
Copy link
Member

Yup, I had the same concern with this.
Here is the reason for GFortran being called:

lfortran/src/bin/lfortran.cpp

Lines 2111 to 2113 in a59bba5

if (compiler_options.po.dump_fortran) {
arg_backend = "fortran";
}

@certik
Copy link
Contributor Author

certik commented Oct 27, 2023

I see. We'll have to fix it. The backend should be the same as selected by the other options. The --dump-all-passes-fortran should just dump all the passes, but not change the backend.

@Shaikh-Ubaid Shaikh-Ubaid self-assigned this Oct 27, 2023
@Thirumalai-Shaktivel
Copy link
Member

Thirumalai-Shaktivel commented Oct 27, 2023

Regarding the errors in the Fortran backend, I made some good progress there earlier today. I will push them tomorrow.

@certik
Copy link
Contributor Author

certik commented Oct 27, 2023

The GFortran failure is caused by #2746, that's unrelated.

@Shaikh-Ubaid
Copy link
Member

Also, there is a slight difference between how the ASR to tree/json/viz and how the ASR to fortran flags work.

For example, we use lfortran examples/expr2.f90 --show-asr --tree/json/visualize to generate ASR in respective format. To apply ASR passes or to generate ASR output for each pass the API or flags we use are lfortran examples/expr2.f90 --show-asr --tree/json/visualize --dump-all-passes.

But for generating ASR as fortran code we have: lfortran examples/expr2.f90 --show-fortran and lfortran examples/expr2.f90 --show-fortran --dump-all-passes-fortran which seems to differ from the previous API.

I would ideally want to somehow unify these for example: lfortran examples/expr2.f90 --show-asr --fortran --dump-all-passes. But the concern is that fortran is backend, where as tree/json/viz are different forms of ASR.

Do you have any suggestions/ideas on this?

@Shaikh-Ubaid
Copy link
Member

Regarding the errors in the Fortran backend, I made some good progress there earlier today. I will push them tomorrow.

@Thirumalai-Shaktivel do you mean you are working on this current issue? If that is the case, please assign it and I can tackle some other issue.

@Thirumalai-Shaktivel
Copy link
Member

Nope, go ahead work on it and submit a PR.

@Shaikh-Ubaid
Copy link
Member

There is also another bug/todo thing with the lfortran examples/expr2.f90 --show-asr --tree/json/viz --dump-all-passes that it generates the respective tree/json/viz pass outputs but along with it also generates the text format output (after each pass).

@certik
Copy link
Contributor Author

certik commented Oct 27, 2023

I would ideally want to somehow unify these for example: lfortran examples/expr2.f90 --show-asr --fortran --dump-all-passes. But the concern is that fortran is backend, where as tree/json/viz are different forms of ASR.

Do you have any suggestions/ideas on this?

Yes. The Fortran backend should be treated like the C backend or LLVM backend. It applies some passes, then generates code. The C backend must compile with Clang/GCC, the Fortran backend must compile with GFortran/ifort, etc. We can make it configurable exactly the level of support, but in general (and the default) it should produce such C or Fortran code that compiles with all compilers. For example, the Fortran can't use any LFortran extensions such as templates/generics, dicts, lists, sets, unsigned integers, etc. Rather, it must apply ASR->ASR passes to rewrite these features into such ASR that can be understood by GFortran. Conclusion: backends apply ASR passes, then generate code for a subset of ASR.

Now let's talk about visualizing ASR. Those ASR visualizers can visualize any ASR node directly, they do not need any ASR passes to be called. They support all of ASR in a straightforward (single pass) manner. Examples of such visualizes are the default (Clojure/LISP like) output, JSON, the text and binary serialization, etc. And Fortran, and later Python (LPython). In principle any high-level language backend (C, C++, Julia) can graduate to such "visualization" backend, where it can generate code for any ASR. Right now we are only focusing on Fortran. The Fortran "visualization" should work for any ASR, without applying any ASR passes, and it should produce code that LFortran can ingest back (eventually).

Note: the Fortran visualizer will not be 100% round-trippable: some information is lost (such as details about physical types). However, if we wanted, probably in principle we can store even such information using some pragmas and teach LFortran to read it.

@Shaikh-Ubaid
Copy link
Member

Rather, I would expect that --dump-all-passes-fortran just dumps the ASR passes as Fortran code, but otherwise does whatever lfortran would do without this option, which in the case above would just compile and run the code:

For now, in #2749, I made --dump-all-passes-fortran to work exactly like the --dump-all-passes used to work originally.

but otherwise does whatever lfortran would do without this option

The --dump-all-passes originally does not dump ASR outputs after each passes if any task, apart from compilation to a binary is given to LFortran. For example consider lfortran_in_main examples/expr2.f90 --show-fortran --dump-all-passes. Although we specified --dump-all-passes, it still just outputs two .clj files.

I think using --dump-all-passes and --dump-all-passes-fortran with no other flag provided is a good approach for now. #2749 implements the same. Please let me know what you think.

@certik
Copy link
Contributor Author

certik commented Oct 28, 2023

Thanks for fixing it! I will test this in the coming days and report back here if there are other high priority usability issues to fix. If not, I'll close this issue.

@Shaikh-Ubaid
Copy link
Member

Thanks for fixing it! I will test this in the coming days and report back here if there are other high priority usability issues to fix. If not, I'll close this issue.

Sure, Thanks for reporting the issue. I will be happy to update/improve the approach based on the feedback.

I think using --dump-all-passes and --dump-all-passes-fortran with no other flag provided is a good approach for now.

Please note that we just need to provide --dump-all-passes or --dump-all-passes-fortran and no other flags to use the feature. For now this seems to be a good approach as it is in the direction of "one canonical way to do things".

@Thirumalai-Shaktivel
Copy link
Member

I have some doubts:

  • The command lfortran examples/expr2.f90 --dump-all-passes-fortran --backend=fortran doesn't dump the passes output, it should dump the passes output, right?
  • Should we apply passes for the fortran backend, like we do for c?
  • Here, lfortran examples/expr2.f90 --dump-all-passes-fortran --backend=fortran should GFortran compile the plain ASR code or the code after applying the passes?

@certik
Copy link
Contributor Author

certik commented Oct 31, 2023

Answers:

  • I think it should print the output
  • I think we want both --- you sometimes want to apply passes if you want to rewrite some LFortran extensions to a more basic Fortran that other compilers can compile; But other times you want to emit the ASR as is
  • I would make this configurable, which Fortran compilers to use (including LFortran)

@Shaikh-Ubaid
Copy link
Member

For now, the --dump-all-passes-fortran works exactly like the --dump-all-passes works (but just outputs fortran code instead of ASR Pickle). This is kept to avoid confusion between the workings of the two.

Should we apply passes for the fortran backend, like we do for c?

I think we should have the provision to apply the passes (for example using --pass= and --skip-pass=), but should not apply any passes by default.

PS: The --dump-all-passes and --dump-all-passes-fortran are more like developer features, so I think/guess they are a lower priority.

@Thirumalai-Shaktivel
Copy link
Member

@Shaikh-Ubaid, do you want to work on the above answers by @certik? If you are busy with the other issues, let me know. I need them for the Fortran backend.

@Thirumalai-Shaktivel
Copy link
Member

Thirumalai-Shaktivel commented Oct 31, 2023

I think we have to apply the passes by default for the Fortran backend, as we apply passes for both the C and LLVM backends.

Conclusion: backends apply ASR passes, then generate code for a subset of ASR.
#2744 (comment)

@certik concluded it here.

Also, I think the command lfortran examples/expr2.f90 --dump-all-passes-fortran --backend=fortran has to dump all passes output, as we do for dump-all-passes-fortran --show-llvm.
Currently, it only dumps the initial ASR.

@Thirumalai-Shaktivel
Copy link
Member

Regarding configuring the Fortran backend compilers, what would be the best design?
Using:

  • Default would be gfortran, specifying option like --compile-with-ifort or --compile-with-lfortran would change the Fortran compiler
  • --backend=fortran=>gfortran or --backend=fortran=>fort 

@Shaikh-Ubaid
Copy link
Member

Shaikh-Ubaid commented Oct 31, 2023

Thirumalai: do you want to work on the above answers by the @certik ?

I would be happy to. If you haven't noticed, I had already shared about working on the feedback.

Ondrej: Thanks for fixing it! I will test this in the coming days and report back here if there are other high priority usability issues to fix. If not, I'll close this issue.

Ubaid: Sure, Thanks for reporting the issue. I will be happy to update/improve the approach based on the feedback.

I think we should test or experience the current approach for sometime (probably a week or two) and then based on experience/feedback work on further improving it. But if you need it early/immediately I am happy to fix it first.

@certik
Copy link
Contributor Author

certik commented Oct 31, 2023

The --backend=fortran and configure it via other options and environment variables, such as LFORTRAN_FORTRAN_COMPILER=ifort and LFORTRAN_FORTRAN_COMPILER_OPTIONS=-warn all -check all might work. People will use this for projects that only LFortran can compile (for example because we implement some new feature like the templates), and one will enable some passes to simplify the ASR, for example to remove templates. Because we do not control other compilers and they might implement templates later, we just need to be flexible with this and maybe do less initially.

Regarding lfortran examples/expr2.f90 --dump-all-passes-fortran --backend=fortran, indeed it only prints pass_00_initial_code_01.f90 and pass_00_initial_code_02.f90. Does it mean that there are no other passes being run? Or are they not printed (that would be a bug)?

@Thirumalai-Shaktivel
Copy link
Member

Yes, lfortran examples/expr2.f90 --dump-all-passes-fortran --backend=fortran compiles the code of initial ASR without passes being applied (and also the passes output are not printed to the file). It's the same case for --dump-all-passes, I think we need fix that as well.

@Shaikh-Ubaid
Copy link
Member

Yup, both --dump-all-passes-fortran and --dump-all-passes work in a similar way. Both --dump-all-passes-fortran and --dump-all-passes work when they are provided alone. We are yet to handle other cases (when some flags are provided).

@Shaikh-Ubaid
Copy link
Member

@Thirumalai-Shaktivel I would be focusing on this in the next or next-to-next week. Please feel free to make progress on this if you need this early.

@Shaikh-Ubaid
Copy link
Member

@Thirumalai-Shaktivel I am unassigning this for you. I would assign it back in the next or next-to-next week if this is still open.

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 a pull request may close this issue.

3 participants