This is an archive of the discontinued LLVM Phabricator instance.

[flang] CUDA Fortran - part 1/5: parsing
ClosedPublic

Authored by klausler on May 8 2023, 4:59 PM.

Details

Summary

Begin upstreaming of CUDA Fortran support in LLVM Flang.

This first patch implements parsing for CUDA Fortran syntax,
including:

  • a new LanguageFeature enum value for CUDA Fortran
  • driver change to enable that feature for *.cuf and *.CUF source files
  • parse tree representation of CUDA Fortran syntax
  • dumping and unparsing of the parse tree
  • the actual parsers for CUDA Fortran syntax
  • prescanning support for !@CUF and !$CUF
  • basic sanity testing via unparsing and parse tree dumps

... along with any minimized changes elsewhere to make these
work, mostly no-op cases in common::visitors instances in
semantics and lowering to allow them to compile in the face
of new types in variant<> instances in the parse tree.

Because CUDA Fortran allows the kernel launch chevron syntax
("call foo<<<blocks, threads>>>()") only on CALL statements and
not on function references, the parse tree nodes for CallStmt,
FunctionReference, and their shared Call were rearranged a bit;
this caused a fair amount of one-line changes in many files.

More patches will follow that implement CUDA Fortran in the symbol
table and name resolution, and then semantic checking.

Diff Detail

Event Timeline

klausler created this revision.May 8 2023, 4:59 PM
Herald added a project: Restricted Project. · View Herald Transcript
klausler requested review of this revision.May 8 2023, 4:59 PM
clementval accepted this revision.May 9 2023, 9:26 AM

I'm not a CUDA Fortran expert but this looks good to me.

flang/lib/Lower/Allocatable.cpp
371

TODO(loc, "cuda stream"); so it triggers the not yet implemented error.

This revision is now accepted and ready to land.May 9 2023, 9:26 AM

driver change to enable that feature for *.cuf and *.CUF source files or did you added it to lit?

Driver logic LGTM, thanks!

Sorry. I missed the frontend changes.

klausler marked an inline comment as done.May 9 2023, 2:52 PM
klausler updated this revision to Diff 520832.May 9 2023, 3:08 PM

Address review comment about TODO() usage in lowering.

If anybody still wishes to review this patch, please let me know, and I will happily wait for you to do so before I merge it.

Shouldn't an RFC be opened before start upstreaming CUDA stuff.
Is the community OK with supporting this Nvidia/PGI language extension/dialect?
If for example Intel invents a new extension based OneAPI/SYCL, Should Flang supports it?
There should be a policy for this kind of changes.

clementval added a comment.EditedMay 12 2023, 11:30 AM

Shouldn't an RFC be opened before start upstreaming CUDA stuff.
Is the community OK with supporting this Nvidia/PGI language extension/dialect?
If for example Intel invents a new extension based OneAPI/SYCL, Should Flang supports it?
There should be a policy for this kind of changes.

clang has cuda support.

klausler added a comment.EditedMay 12 2023, 11:33 AM

Shouldn't an RFC be opened before start upstreaming CUDA stuff.
Is the community OK with supporting this Nvidia/PGI language extension/dialect?
If for example Intel invents a new extension based OneAPI/SYCL, Should Flang supports it?
There should be a policy for this kind of changes.

clang has cuda support.

And so does Classic Flang.

Note that this particular extension is enabled only by means of a distinct source file suffix.

And yes, I would absolutely welcome support for more Fortran language extensions. We already support as many CDC, Cray, DEC, IBM, Intel, NVIDIA/PGI, and probably other extensions that I know about and that have survived to the present day, and others are busily working on adding more, like IBM PPC vector extensions. The more Fortran applications that LLVM Flang can correctly compile unmodified, the better.

Shouldn't an RFC be opened before start upstreaming CUDA stuff.

CUDA is very widely adopted and I doubt that anyone would object to this. The changes in the driver are very non-intrusive and I see no need to discuss this more widely. Definitely not objecting to an RFC (in fact, a short PSA would be nice), but I'm happy for this to land without further advertising.

If for example Intel invents a new extension based OneAPI/SYCL, Should Flang supports it?

If there is a critical mass of users that would like to use a particular extension then it should be supported. It doesn't matter whether it comes from "company A" or "company B". As for SYCL specifically, isn't that a purely C++ extension? What's the relevance here?

MehdiChinoune added a comment.EditedMay 15 2023, 9:58 PM

There is a big difference between CUDA and CUDA-Fortran.
1 - CUDA could be used by any compiler MSVC, Intel, IBM, GNU... as It doesn't require any support from the compiler. It's just libraries+headers.
While CUDA-Fortran requires a support from the compiler itself.
GCC, Intel, Cray, Fujitsu ... compilers do not support it because It's just an Nvidia/PGI dialect, Flang-Classic is just an open-sourced Frontend of PGI compiler.
2 - CUDA-Fortran requires a proprietary CUDA SDK to be installed which make it different from other dialects/extensions you mentioned above. those are language extensions they don't need any SDK to be installed.

We are talking here about a company pushing for a dialect not widely used by the community.
If you fellow many discussions online in Fortran community, you know for sure that PGI compiler is one of the least/worst used compiler.
I think Nvidia should keep CUDA-Fortran support in the commercial/proprietary compiler that will be based on flang.

MehdiChinoune added a comment.EditedMay 15 2023, 10:04 PM

Could you at least make CUDA-Fortran support behind a CMake option.
As a package maintainer on a platform where CUDA is not supported (I am sure I am not alone), I don't want to fix code that will never benefit my platform.
You know this a community project, CUDA is not supported everywhere.

I'm fairly sympathetic to @MehdiChinoune 's argument here. You're essentially asking an open-source community to support your proprietary technology. Maybe it's widely enough used that it deserves to be (I'm at least open to the argument), but you should definitely be open and upfront about wanting to do that, and ask the community through the proper procedures first. I think it would have a been a violation of the community's trust if it had been approved and made it into the official repository without anybody outside Nvidia having a chance to comment. Maybe I missed it, but I don't remember hearing about this plan before the patches got submitted. And based on the number of reviewers listed here, it doesn't look like you were trying to publicise it very broadly.

There is a big difference between CUDA and CUDA-Fortran.
1 - CUDA could be used by any compiler MSVC, Intel, IBM, GNU... as It doesn't require any support from the compiler. It's just libraries+headers.
While CUDA-Fortran requires a support from the compiler itself.

This is actually not the case. A CUDA C(++) program needs to be compiled by a C(++) compiler capable of handling the same features (function attributes, data attributes, kernel launch, &c.) that are present in CUDA Fortran.

GCC, Intel, Cray, Fujitsu ... compilers do not support it because It's just an Nvidia/PGI dialect, Flang-Classic is just an open-sourced Frontend of PGI compiler.

IBM's XLF Fortran compiler also supports CUDA Fortran.

2 - CUDA-Fortran requires a proprietary CUDA SDK to be installed which make it different from other dialects/extensions you mentioned above. those are language extensions they don't need any SDK to be installed.

It's no different from the CUDA support in Clang or the PTX support in the LLVM code generator in that respect.

We are talking here about a company pushing for a dialect not widely used by the community.
If you fellow many discussions online in Fortran community, you know for sure that PGI compiler is one of the least/worst used compiler.
I think Nvidia should keep CUDA-Fortran support in the commercial/proprietary compiler that will be based on flang.

I would much rather have our source in the open and available to all, which is why LLVM Flang exists in the first place rather than being a closed-source NVIDIA project to improve its Fortran product.

I'm fairly sympathetic to @MehdiChinoune 's argument here. You're essentially asking an open-source community to support your proprietary technology. Maybe it's widely enough used that it deserves to be (I'm at least open to the argument), but you should definitely be open and upfront about wanting to do that, and ask the community through the proper procedures first. I think it would have a been a violation of the community's trust if it had been approved and made it into the official repository without anybody outside Nvidia having a chance to comment. Maybe I missed it, but I don't remember hearing about this plan before the patches got submitted. And based on the number of reviewers listed here, it doesn't look like you were trying to publicise it very broadly.

I've left these reviews open for comment for a long time, and will continue to do so to collect relevant comments from people who actually participate in the project.

Could you at least make CUDA-Fortran support behind a CMake option.
As a package maintainer on a platform where CUDA is not supported (I am sure I am not alone), I don't want to fix code that will never benefit my platform.
You know this a community project, CUDA is not supported everywhere.

Hi Mehdi,

It's a good idea to guard CUDA Fortran dependencies on CUDA in a similar way to how clang's CUDA C++ dependencies on CUDA are guarded.

This patch is front end work; because there's no lowering yet, there's no need to add such CMake options yet. The feature is gated from Fortran users by way of a special required file extension (.cuf).

CUDA can work with Fortran using modules, interfaces, and libraries similarly to how CUDA can work with C++ using headers and libraries. But both CUDA C++ and CUDA Fortran are language extensions, e.g. chevron syntax.

Generally regarding extensions, the Flang policy is spelled out in https://github.com/llvm/llvm-project/blob/48c3ae5cc3d4612283018ea597db3f7214aabfd9/flang/docs/Extensions.md.

As a general principle, this compiler will accept by default and without complaint many legacy features, extensions to the standard language, and features that have been deleted from the standard, so long as the recognition of those features would not cause a standard-conforming program to be rejected or misinterpreted.

Adding support for CUDA Fortran follows both this spirit and intent as did the addition of OpenMP and OpenACC.

Hi Mehdi,

It's a good idea to guard CUDA Fortran dependencies on CUDA in a similar way to how clang's CUDA C++ dependencies on CUDA are guarded.

This patch is front end work; because there's no lowering yet, there's no need to add such CMake options yet. The feature is gated from Fortran users by way of a special required file extension (.cuf).

I was talking about flang code. These changes could make flang fails to build. So we have to fix it because a feature we don't use.

Adding support for CUDA Fortran follows both this spirit and intent as did the addition of OpenMP and OpenACC.

OpenMP and OpenACC are open standards.

I'm fairly sympathetic to @MehdiChinoune 's argument here. You're essentially asking an open-source community to support your proprietary technology. Maybe it's widely enough used that it deserves to be (I'm at least open to the argument), but you should definitely be open and upfront about wanting to do that, and ask the community through the proper procedures first. I think it would have a been a violation of the community's trust if it had been approved and made it into the official repository without anybody outside Nvidia having a chance to comment. Maybe I missed it, but I don't remember hearing about this plan before the patches got submitted. And based on the number of reviewers listed here, it doesn't look like you were trying to publicise it very broadly.

I've left these reviews open for comment for a long time, and will continue to do so to collect relevant comments from people who actually participate in the project.

But my point was that you have not actually solicited feedback from everybody who "actually participates," only those actively monitoring every submission to Phabricator. Some people can't actively monitor every submission, since many people contribute on a voluntary or part time basis, and I think their opinions matter too.

I was talking about flang code. These changes could make flang fails to build. So we have to fix it because a feature we don't use.

I couldn't find any CMakeLists.txt configuration enablement switches for CUDA in Clang.

klausler updated this revision to Diff 523551.May 18 2023, 2:05 PM

Rebase to current head.

Is CUDA Fortran an extrension of ISO Fortran? Do you need to mention it in Extensions.md?

I believe it is fine to support CUDA Fortran in Flang. This is broadly in line with CUDA support in Clang. I believe there is sufficient documentation in the open that specifies its syntax and semantics (https://docs.nvidia.com/hpc-sdk/compilers/cuda-fortran-prog-guide/index.html). Also, it can be seen that we included cuf as part of the file extensions that we wanted to test initially.

I am not a CUDA Fortran expert but the patch looks OK to me.

klausler updated this revision to Diff 526721.May 30 2023, 11:06 AM

Rebase to current llvm-project/main in preparation for merging.

This revision was automatically updated to reflect the committed changes.