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

flang-new -> flang renaming article #62

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

DavidSpickett
Copy link
Contributor

@DavidSpickett DavidSpickett commented Nov 14, 2024

Note: This article is to be published just after the release of LLVM 20, around Feburary 2025. Which will be the first release that includes LLVM Flang as flang instead of flang-new.

The original proposal being https://discourse.llvm.org/t/proposal-rename-flang-new-to-flang/69462.

I proposed writing this article (https://discourse.llvm.org/t/rfc-llvm-project-blog-post-for-flang-new-flang-renaming/80915) to mark that renaming.

It covers:

  • Why fortran still exists.
  • Why you would build a new compiler.
  • Flang's history.
  • How the community decided to rename it.
  • Flang's adoption of MLIR.
  • Quotes from users/contributors/etc.
  • How to contribute to Flang.

@DavidSpickett
Copy link
Contributor Author

DavidSpickett commented Nov 14, 2024

I am posting this very early relative to the likely February publishing date so we have plenty of time to gather feedback with all the holidays coming up.

It is ready for review, though the main focus at the moment is factual accuracy and general layout. I will concentrate on grammar and phrasing later, I have some reviewers in mind for that. I suggest you use the "rich" diff view so you can read the rendered Markdown document.

I am also collecting quotes for the article. Some of you I will be emailing directly, but anyone can be included and you can comment here instead. I cannot guarantee all input will be used but you of course have the right to ask that a quote be removed or corrected or put in the proper context if it is used.

(reading or reviewing the whole article is not required to give a quote, it's here for inspiration if you need it)

Quotes can be about anything related to the project:

  • Could be something that Flang allowed you to build
  • work that you enjoyed doing on Flang
  • something you think Flang has set a new standard for
  • how Flang contributes to your company/institution's strategy.

The goal is to show the breadth of the community and all the unique things people have got/continue to get from this project.

Please keep your quotes to a few sentences if possible.

If you have a particular way you wish to be cited, please let me know. My default is what I see on GitHub and the git log, plus the company name. If you want something different please tell me exactly what you prefer it to be, include:

  • Any special characters or formatting.
  • Translated forms of the name, e.g Japanese Kanji and a Romanised version, I'll include both.
  • Your job title if you want that included
  • Your company name, if you want it included

@DavidSpickett DavidSpickett marked this pull request as ready for review November 14, 2024 11:15
Copy link

@banach-space banach-space left a comment

Choose a reason for hiding this comment

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

Thanks, David. This reads really well, and it’s clear you’ve put in a lot of detective work to gather all this information - it’s greatly appreciated!

I’ve left a few minor comments and suggestions. After reading through, I see that you actually address most of these later in the text :)

I’ll aim to contribute something on the driver in the next few days.

content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
@DavidSpickett
Copy link
Contributor Author

I’ve left a few minor comments and suggestions. After reading through, I see that you actually address most of these later in the text :)

These sort of comments are good to. Sometimes it's good for the reader to have questions for later, but if it's something they're going to be distracted by as they read, I should address that.

@DavidSpickett
Copy link
Contributor Author

FYI: big updates to the FIR and HLFIR sections coming, you can still point out my mistakes though :)

Copy link

@luporl luporl left a comment

Choose a reason for hiding this comment

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

I added a few nits, but the MLIR part looks good to me.

content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
@DavidSpickett
Copy link
Contributor Author

DavidSpickett commented Nov 19, 2024

I added a few nits, but the MLIR part looks good to me.

Thanks Leandro! I will be re-ordering the section soon, after I have more input from some Nvidia folks but I expect the factual content to be the same.

content/posts/2024-11-05-flang-new.md Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
content/posts/2024-11-05-flang-new.md Outdated Show resolved Hide resolved
Copy link

@luporl luporl left a comment

Choose a reason for hiding this comment

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

Great explanation of OpenMP basics, illustrated with how Clang translates it to LLVM IR and contrasted with Flang, that first lowers it to MLIR.

Comment on lines +461 to +462
The compiler interprets these directives and produces calls to a runtime library
that distributes work as instructed.
Copy link

Choose a reason for hiding this comment

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

At first glance, this gives me the impression that the compiler just needs to emit calls to a runtime library to distribute the work. But in most cases the block of code to be parallelized needs to be outlined to a separate function, copies of variables must be made, and other setup steps need to be performed.

Maybe something like the following would be more accurate:
The compiler interprets these directives and, by performing transformations in the marked code and producing calls to a runtime library, distributes work as instructed.

```
([Compiler Explorer](https://godbolt.org/z/eca6vjbqe))

Instead of `!`, comments start with `#` and instead of `DO` there is `for`.
Copy link

Choose a reason for hiding this comment

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

C/C++ actually uses #pragma directives instead of comments.

Overall very similar to the Fortran example.

LLVM IR does not know anything about OpenMP specifically, so Clang uses a
library called `OpenMPIRBuilder` to convert OpenMP directives to LLVM IR.
Copy link

Choose a reason for hiding this comment

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

Pedantic comment: the OpenMPIRBuilder class is actually part of the LLVMFrontendOpenMP library.

Comment on lines +508 to +528
define dso_local void @simple(<...>) {
<...>
call void <...> @__kmpc_fork_call(<...> ptr @simple(<...>) (.omp_outlined), <...>
ret void
}

define internal void @simple(<...>) (.omp_outlined) <...>) {
<...>
call void @simple(<...>) (.omp_outlined_debug__) <...>
ret void
}

define internal void @simple(<...>) (.omp_outlined_debug__) <...> {
<...>
call void @__kmpc_for_static_init_4(ptr @1, i32 %8, i32 34, <...>)
<...>
omp.inner.for.body:
<...>
omp.precond.end:
ret void
}
Copy link

Choose a reason for hiding this comment

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

It took me some time to realize that this is the output of clang and not flang, although it may be lack of attention on my side, as the previous paragraph talks about clang.

Also, why -O3 was used in the Fortran example but not in the C++ one?
With -O3, only 2 versions of simple are generated, which can simplify the example.

ret void
}

define internal void @simple(<...>) (.omp_outlined) <...>) {
Copy link

Choose a reason for hiding this comment

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

Parentheses are not balanced here.

You will see that instead of `for.do_loop` you have `omp.parallel`. `omp` is
MLIR's [OpenMP dialect](https://mlir.llvm.org/docs/Dialects/OpenMPDialect/).
This is a higher level and much more literal translation of the
`parallel` directive than Clang produced.
Copy link

Choose a reason for hiding this comment

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

Suggested change
`parallel` directive than Clang produced.
`parallel do` combined directive than Clang produced.


**Note:** Fortran arrays are [one-based](https://fortran-lang.org/en/learn/quickstart/arrays_strings/) by default. So the first element is at index 1. This example reads the previous element, so it starts `I` at 2.

`!$OMP PARALLEL DO` is a directive in the form of a Fortran comment, which
Copy link

Choose a reason for hiding this comment

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

Suggested change
`!$OMP PARALLEL DO` is a directive in the form of a Fortran comment, which
`!$OMP PARALLEL DO` is a combined directive in the form of a Fortran comment, which

}
```

There are some differences between this and the IR Clang produced but
Copy link

Choose a reason for hiding this comment

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

Adding -O3 to Clang example reduces the differences.


## ClangIR and the Future

It is surprising that a compiler for a language as old as Fotran got ahead of
Copy link

Choose a reason for hiding this comment

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

Suggested change
It is surprising that a compiler for a language as old as Fotran got ahead of
It is surprising that a compiler for a language as old as Fortran got ahead of

above:

```mlir
define void @simple_(<...> {
Copy link

Choose a reason for hiding this comment

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

nit: this is a matter of preference (so feel free to ignore it), but the omitted closing parentheses in this snippet makes it harder for me to parse it.

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.

3 participants