[flang] Version information in flang/f18

[coti]

Summary

Fixed some version information in flang/f18:

• • fixed the behavior of the -v switch: this flag enables verbosity with used with arguments, but just displays the version when used alone (related to this bug: https://bugs.llvm.org/show_bug.cgi?id=46017)
• added FLANG, FLANG_MAJOR, FLANG_MINOR__ and FLANG_PATCHLEVEL (similar to their __F18* counterparts) for compatibility purpose

[AlexisPerry]

Thanks for the patch! I had a couple questions and one small change request (see inline comments). The questions truly should be treated as questions rather than requests for changes, since I'm not 100% sure of the right answer to them.

flang/tools/f18/f18.cpp

390

Should we use "flang" instead of "f18" to better align with the project's name?

414–417

Should these replace the __F18 equivalents rather than add on? Also, should they match the clang/llvm version numbers, which I believe are 12.0.0 now?

634

Please change to arg == "--version" to align with the clang, gfortran, and pgf90 flag.

[coti]

Applied suggestions from Alexis.

[AlexisPerry]

Looks good to me. Please wait for approval from at least one of the other reviewers before merging.

[sscalpone]

flang/tools/f18/f18.cpp

410

Cmake has the capability to generate files, usually taking a "foo.in" and creating a "foo" with macro substitution. Is that technique possible for the version numbers?

[klausler]

flang/tools/f18/f18.cpp

414

The older flang compiler reportedly defines __FLANG. If so, it might be a bad idea to also define __FLANG in this newer flang compiler,; how would conditionally compiled code distinguish one flang compiler from the other?

[coti]

Answered some inline comments.

flang/tools/f18/f18.cpp

390

Good question. I left the original message, I let senior developers make a decision here.

410

Yes it is! Should I "update diff" to show it?

414

So I have a question here: what is the rule regarding compatibility between old and new flang? I came across situations where I needed these because I tried to compile software that knew the old flang and used these values.

[klausler]

flang/tools/f18/f18.cpp

414

Does that software that you mentioned use #ifdef __FLANG or #if __FLANG, as opposed to checking the value of the macro?

[coti]

Replied inline comment from @klausler

flang/tools/f18/f18.cpp

414

This is in SuperLU.

It looks for all of them but __FLANG only needs to be defined:

#elif defined(__FLANG)
        PRINT *, 'INFO:compiler[Flang]'
# define COMPILER_VERSION_MAJOR DEC(__FLANG_MAJOR__)
# define COMPILER_VERSION_MINOR DEC(__FLANG_MINOR__)
# if defined(__FLANG_PATCHLEVEL__)
#  define COMPILER_VERSION_PATCH DEC(__FLANG_PATCHLEVEL__)
# endif

[coti]

Fixed an error in my reply

flang/tools/f18/f18.cpp

414

Sorry, not in SuperLU, but in CMake 3.17.0
https://gitlab.kitware.com/cmake/cmake/blob/master/Modules/CMakeFortranCompilerId.F.in

[coti]

Let cmake grab the version numbers.

[naromero77]

I am assuming that f18 will have a MAJOR version number matching the LLVM release? Does classic flang do that as well? Both compilers will probably co-exist for a while and it would be desirable to make them distinguishable to build systems as CMake.

I don't have a solution, but maybe we need to #define __CLASSIC_FLANG in classic flang. I am just throwing out ideas.

[AlexisPerry]

In D84334#2173076, @naromero77 wrote:

I am assuming that f18 will have a MAJOR version number matching the LLVM release? Does classic flang do that as well? Both compilers will probably co-exist for a while and it would be desirable to make them distinguishable to build systems as CMake.

I don't have a solution, but maybe we need to #define __CLASSIC_FLANG in classic flang. I am just throwing out ideas.

I agree with this idea, and if we went this route we could get rid of the duplicate code between __F18_MAJOR and __FLANG_MAJOR (and the others) included in this patch. We would have to coordinate with the Classic Flang developers to make it happen, though, and I'm not sure if they are aware of the discussion happening here.

[gak]

In D84334#2176184, @AlexisPerry wrote:

In D84334#2173076, @naromero77 wrote:

I am assuming that f18 will have a MAJOR version number matching the LLVM release? Does classic flang do that as well? Both compilers will probably co-exist for a while and it would be desirable to make them distinguishable to build systems as CMake.

I don't have a solution, but maybe we need to #define __CLASSIC_FLANG in classic flang. I am just throwing out ideas.

I agree with this idea, and if we went this route we could get rid of the duplicate code between __F18_MAJOR and __FLANG_MAJOR (and the others) included in this patch. We would have to coordinate with the Classic Flang developers to make it happen, though, and I'm not sure if they are aware of the discussion happening here.

At least one is. I think the idea of defining __CLASSIC_FLANG in the compiler is a good one.

Classic Flang's version numbering started out unrelated to the LLVM version numbering. I am not sure how we should change it to align yet, but it's something we can look into.

[richard.barton.arm]

Although LLVM flang is the spiritual successor to classic flang, I don't think we are specifically aiming for any sort of compatibility between the two compilers, for example on option names, feature support, language support, processor defined behavior, 'bug compatibility', etc. so in this sense the two are completely different compilers. I think it unlikely that a user of classic flang that has conditionalized some code on __FLANG due to some quirk of classic flang or some bug in classic flang that they are working around will necessarily need or want to run the same code for LLVM flang for the same reason. I think this would steer us strongly away from supporting __FLANG in LLVM flang to indicate that it and classic flang are connected in this way.

Could we simply follow what clang does here and use lowercase for the macro name, i.e. __flang_major__ ? That would make LLVM flang totally distinct from classic flang whilst also preserving the obvious meanings of the macros.

I would also suggest matching clang a bit more closely, so using the trailing underscores across the piece and also making __flang__ defined to 1 and adding __flang_version__ to hold the full version number.

I think keeping __F18 for backwards compatibility is fine, but given that the community today is very small (just the developers) if we can take this opportunity to delete it now before more users can come to depend on it then we should do so. I guess the only likely existing users might be @AlexisPerry or someone in @sscalpone 's team?

My proposal would be to support

#define __flang__            1 
#define __flang_major__      "@LLVM_VERSION_MAJOR@"
#define __flang_minor__      "@LLVM_VERSION_MINOR@"
#define __flang_patchlevel__ "@LLVM_VERSION_PATCH@"
#define __flang_version__    "@LLVM_VERSION_MAJOR@.@LLVM_VERSION_MINOR@.@LLVM_VERSION_PATCH@"

+ the existing __F18... flags for backwards compatibility if we absolutely need to.

I also think we should ask to cherry pick this to the LLVM 10 release branch.

Thanks for the patch @coti , we should have thought of this sooner :-)

[AlexisPerry]

@richard.barton.arm, I don't think I have any code that relies on __F18 being defined, so I'm alright with removing it and the related macros. You also make a very good point about following clang behavior. I wasn't aware of clang's use of lowercase macros and just assumed everyone uses uppercase macros based on my previous experiences with other codes. I think your proposal is likely the better way to go, first because matching style across LLVM projects is generally a good thing and second because your proposal does not require changes to classic Flang so we don't have to impose on those developers.

Also, +1 on cherry picking this patch to the LLVM 11 release branch once it is merged.

flang/tools/f18/f18.cpp

392

If we adopt @richard.barton.arm's proposal, I think it would be a good idea to make use of the __flang_version__ macro here so that the output actually prints the version number.

[coti]

I have made the modifications suggested by @richard.barton.arm, but then, CMake does not compile anymore because it relies on __FLANG, __FLANG_MAJOR__, __FLANG_MINOR__ and __FLANG_PATCHLEVEL__.

I have two options there:

• define them, instead of their __F18_*__ counterparts, since @AlexisPerry suggests removing the latter
• submit a patch to CMake, which might take a bit longer. For the record, the support for (classic) flang in CMake was documented here https://gitlab.kitware.com/cmake/cmake/-/issues/17173

[richard.barton.arm]

I don't think it is safe to assume that LLVM Flang will always be the same as classic Flang for all of the same behaviors that CMake cares about. I think we should be considering LLVM Flang as a new compiler rather than as a different version of classic Flang. I think that means we need to make a change in CMake to add support for LLVM Flang as a new compiler.

[coti]

Here is a new version with, as suggested by @richard.barton.arm:

• removed __F18*
• added __flang*

Also:

• The version numbers are obtained from the project by CMake
• As suggested by @AlexisPerry, flang -v displays the version number

[isuruf]

flang/tools/f18/CMakeLists.txt

64

Can you use CMAKE_CURRENT_BINARY_DIR to configure the file? Source directory should be read only and not written to.

[coti]

Use CMAKE_CURRENT_BINARY_DIR as the target directory of the geneated .h file, as suggested by @isuruf

[richard.barton.arm]

Looking good. Please can you add some tests?

• For --version, there already is one for the version screen in flang/test/Driver that could be updated to capture the version number also and run multiple times to test the synonyms.
  • Testing {{\-v}} when it means verbose is hard so suggest not doing that. We need {{-###}} really to test the driver more, which is outside the scope of your change.
• For the macros, I think the flang/test/Preprocessing" directory would be the best place but that directory is full of non-tests (which we need to delete really) so we can't add a real test there at the moment. Perhaps add a new test to flang/test/Driver for now that runs flang with -E and checks the substitutions work.

[tskeith]

In D84334#2192984, @richard.barton.arm wrote:

• For the macros, I think the flang/test/Preprocessing" directory would be the best place but that directory is full of non-tests (which we need to delete really) so we can't add a real test there at the moment. Perhaps add a new test to flang/test/Driver for now that runs flang with -E and checks the substitutions work.

The preprocessing tests are being executed now so that would be a good place to add a test.

[klausler]

In D84334#2192984, @richard.barton.arm wrote:

Looking good. Please can you add some tests?

• For --version, there already is one for the version screen in flang/test/Driver that could be updated to capture the version number also and run multiple times to test the synonyms.
  • Testing {{\-v}} when it means verbose is hard so suggest not doing that. We need {{-###}} really to test the driver more, which is outside the scope of your change.
• For the macros, I think the flang/test/Preprocessing" directory would be the best place but that directory is full of non-tests (which we need to delete really) so we can't add a real test there at the moment. Perhaps add a new test to flang/test/Driver for now that runs flang with -E and checks the substitutions work.

Please don't delete the tests in flang/test/Preprocessing.

[richard.barton.arm]

The preprocessing tests are being executed now so that would be a good place to add a test.

Ha - if only I had written that comment yesterday it would have been right :-) Thanks for enabling them @klausler and @tskeith for the correction.

[coti]

I am writing the tests.

I have a little problem with` flang_version. It looks like f18 considers it as a numeric constants that it cannot interpret (probably because it contains too many dots). For example:

print *, "VERSION", __flang_version__

Gives me:

/home/users/coti/toto.f90:10:4: error: expected execution part construct
    print *, "VERSION", __flang_version__
     ^
/home/users/coti/toto.f90:10:3: in the context: execution part
    print *, "VERSION", __flang_version__
    ^
/home/users/coti/toto.f90:1:1: in the context: main program
  program version
  ^
f18: could not parse /home/users/coti/toto.f90

Then

#if __flang_version__ == "12.0.0"
  print *, "Correct version number"
#endif

gives me:

/home/users/coti/toto.f90:15:5: error: Uninterpretable numeric constant '12.0.0'
  #if __flang_version__ == "12.0.0"
      ^^^^^^^^^^^^^^^^^
in a macro defined here
that expanded to:
  12.0.0
  ^

[coti]

Some tests for the -v flag and associated macros.

Some comments:

• I could not find a way to exploit __flang_version__, since it is interpreted as a numeric constant instead of a string (see above)
• The version number is simply matched against a regex. I can retrieve it from cmake, similarly to what is done in f18.cpp, but I chose to keep things simple and not add anything into flang/test/CMakeLists.txt. What do you think?

[richard.barton.arm]

Thanks for adding the tests. IMO, the regex approach in the test is just fine.

I have made a couple of small suggestions and am happy to approve on the basis that those are addressed.

I think one of @AlexisPerry or @sscalpone should also approve before you push.

Thanks for the work on this.

flang/test/Preprocessing/macros.F90

1 ↗

(On Diff #283086)

Suggest this be renamed "compiler_defined_macros" or "predefined_macros" to separate it from a general test on user-defined macros.

flang/tools/f18/f18.cpp

418

Given the bug that you have found with flang_version, I suggest we don't define the external macro in this patch, given that it won't be usable and it's essentially exposing the bug that wasn't there before.

[tskeith]

flang/test/Preprocessing/macros.F90

7 ↗

(On Diff #283086)

Does this depend on having gfortran available? It should use %f18 -E like the others.

[sscalpone]

Here are the clang definitions:

% clang -E -dM -x c /dev/null | grep clang
#define clang 1
#define clang_major 12
#define clang_minor 0
#define clang_patchlevel 0
#define clang_version "12.0.0 (ssh://[e-mail]:12051/fortran/fir-dev-mirror.git 36a44dbf0b4e55917b70fbf209d8c992249e651b)"

flang/tools/f18/f18.cpp

392

Perhaps the message could be "flang front end" ?

[coti]

All the comments should be addressed:

• renamed the test for pre-processor macros compiler_defined_macros.F90
• removed __flang_version__ (in f18.cpp and in the tests)
• use %f18 -E in the pre-processor test in order to avoid a full compilation

[tskeith]

flang/test/Preprocessing/compiler_defined_macros.F90

4

I don't know if you can count on grep being available everywhere. I think FileCheck is better anyway. E.g. something like this:

!CHECK: flang_major = {{[0-9]+$}}
integer, parameter :: flang_major = __FLANG_MAJOR__

[coti]

Thank you @tskeith for the tip on the tests!

[tskeith]

Thanks, looks good to me.

[richard.barton.arm]

Do you have commit access @coti ?

[coti]

No, I don't ;)

[richard.barton.arm]

It needs rebasing - please can you do this then I will commit it on your behalf?

[coti]

Updated my local clone, fixed the conflicts and re-generated the diff.

[richard.barton.arm]

I'm afraid that still doesn't work. My master is on 7d0f691.

[coti]

I generated the diff again on a clean clone. I don't know why but my CMakeList.txt was wrong even after a git stash. Sorry about that, I hope it should be fine now.

[richard.barton.arm]

I'm afraid arcanist does not apply the patch well. It does not like the new file creation. I'm not sure how you are creating this diff, but are you able to re-upload a diff created with diff -Naur to handle the new files correctly?

$ arc patch D84334 --nobranch
 INFO  Base commit is not in local repository; trying to fetch.
Checking patch flang/tools/f18/f18_version.h.in...
error: flang/tools/f18/f18_version.h.in: does not exist in index
Checking patch flang/tools/f18/f18.cpp...
Checking patch flang/tools/f18/CMakeLists.txt...
Checking patch flang/test/Preprocessing/compiler_defined_macros.F90...
error: flang/test/Preprocessing/compiler_defined_macros.F90: does not exist in index
Checking patch flang/test/Driver/version_test.f90...
Applied patch flang/tools/f18/f18.cpp cleanly.
Applied patch flang/tools/f18/CMakeLists.txt cleanly.
Applied patch flang/test/Driver/version_test.f90 cleanly.

 Patch Failed! 
Usage Exception: Unable to apply patch!

[coti]

Outch, sorry :(
I create my patches with git diff....

Thank you for showing me the arc command, this time I checked:

$ arc patch --patch ~/f18_version.patch 
 INFO  Base commit is not in local repository; trying to fetch.
Branch name arcpatch already exists; trying a new name.
Branch name arcpatch_1 already exists; trying a new name.
Branch name arcpatch_2 already exists; trying a new name.
Created and checked out branch arcpatch_3.
Checking patch flang/test/Driver/version_test.f90...
Checking patch flang/test/Preprocessing/compiler_defined_macros.F90...
Checking patch flang/tools/f18/CMakeLists.txt...
Checking patch flang/tools/f18/f18.cpp...
Checking patch flang/tools/f18/f18_version.h.in...
Applied patch flang/test/Driver/version_test.f90 cleanly.
Applied patch flang/test/Preprocessing/compiler_defined_macros.F90 cleanly.
Applied patch flang/tools/f18/CMakeLists.txt cleanly.
Applied patch flang/tools/f18/f18.cpp cleanly.
Applied patch flang/tools/f18/f18_version.h.in cleanly.
Usage Exception: User aborted the workflow.

[richard.barton.arm]

Please can you send me your email address offline for the commit log?

[coti]

yes ;)

[sscalpone]

This commit has broken the out of tree builds.

llvm-project/flang/tools/f18/f18.cpp:41:10: fatal error: f18_version.h: No such file or directory
   41 | #include "f18_version.h"
      |          ^~~~~~~~~~~~~~~

[isuruf]

In D84334#2208010, @sscalpone wrote:

This commit has broken the out of tree builds.

This probably needs to include CMAKE_CURRENT_BINARY_DIR as a include directory.

[echristo]

I'm also not sure this is passing?

---

FAIL: Flang :: Preprocessing/compiler_defined_macros.F90 (31682 of 71064)

• TEST 'Flang ::

Preprocessing/compiler_defined_macros.F90' FAILED ****

Script:

...

Exit Code: 1

Command Output (stderr):

/usr/local/google/home/echristo/sources/llvm-project/flang/test/Preprocessing/compiler_defined_macros.F90:3:9:
error: CHECK: expected string not found in input
!CHECK: flang_major = {{[1-9][0-9]*$}}

^

<stdin>:1:1: note: scanning from here
integer, parameter :: flang_major = flang_major
^
<stdin>:1:23: note: possible intended match here
integer, parameter :: flang_major = flang_major

^

Input file: <stdin>
Check file:
/usr/local/google/home/echristo/sources/llvm-project/flang/test/Preprocessing/compiler_defined_macros.F90

-dump-input=help explains the following input dump.

Input was:
<<<<<<

1: integer, parameter :: flang_major = __flang_major__

check:3'0 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no
match found
check:3'1 ? possible
intended match

2: integer, parameter :: flang_minor = __flang_minor__

check:3'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

3: integer, parameter :: flang_patchlevel = __flang_patchlevel__

check:3'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

--

[richard.barton.arm]

I have reverted the patch

[richard.barton.arm]

@coti - I've had another look and I think your new test is stumbling across an existing bug. If you play around with the existing __F18* macros, you can see that they are not being applied correctly in the code:

ricbar01@hw-a21-17:~/ossf18/llvm-project/flang$ cat test.f90
PROGRAM FOO
  integer i = __F18__
end program foo
ricbar01@hw-a21-17:~/ossf18/llvm-project/flang$ ../../build/bin/flang -DFOO=bar test.f90 -E
program foo
integer i = __f18__
end program foo

So I don't think there is anything wrong with the code changes or test, but perhaps another bug to fix in the preprocessor itself. Its the end of the day for me now, but I'll look again tomorrow.

I have not looked at the out of tree build issues. Have you tried isuru's suggestion?

[klausler]

In D84334#2213327, @richard.barton.arm wrote:

@coti - I've had another look and I think your new test is stumbling across an existing bug. If you play around with the existing __F18* macros, you can see that they are not being applied correctly in the code:

ricbar01@hw-a21-17:~/ossf18/llvm-project/flang$ cat test.f90
PROGRAM FOO
  integer i = __F18__
end program foo
ricbar01@hw-a21-17:~/ossf18/llvm-project/flang$ ../../build/bin/flang -DFOO=bar test.f90 -E
program foo
integer i = __f18__
end program foo

So I don't think there is anything wrong with the code changes or test, but perhaps another bug to fix in the preprocessor itself. Its the end of the day for me now, but I'll look again tomorrow.

I have not looked at the out of tree build issues. Have you tried isuru's suggestion?

I think I remember fixing something in that area with predefined (command-line -D flag) options; I'll see whether it still needs to be pushed.

[coti]

Added include ${CMAKE_CURRENT_BINARY_DIR}, as suggested by @isuruf. I tested it on a clean tree, it seems to work.

[coti]

Typo in a comment.

[DavidTruby]

flang/tools/f18/CMakeLists.txt

33

as a small nit: I think target_include_directories(f18 ${CMAKE_CURRENT_BINARY_DIR}) is slightly cleaner. I'm not that fussed about it though

[coti]

@DavidTruby so, how about replacing:

set(include ${FLANG_BINARY_DIR}/include/flang)

set(include ${FLANG_BINARY_DIR}/include/flang)
set(include ${CMAKE_CURRENT_BINARY_DIR})

with:

target_include_directories(f18
  ${CMAKE_CURRENT_BINARY_DIR}
  ${FLANG_BINARY_DIR}/include/flang 
)

[DavidTruby]

In D84334#2214441, @coti wrote:

@DavidTruby so, how about replacing:

set(include ${FLANG_BINARY_DIR}/include/flang)

set(include ${FLANG_BINARY_DIR}/include/flang)
set(include ${CMAKE_CURRENT_BINARY_DIR})

with:

target_include_directories(f18
  ${CMAKE_CURRENT_BINARY_DIR}
  ${FLANG_BINARY_DIR}/include/flang 
)

looks good to me!

[sscalpone]

This worked for me:

target_include_directories(f18
  PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
)

And I got rid of the duplicate:

set(include ${FLANG_BINARY_DIR}/include/flang)

flang/tools/f18/CMakeLists.txt

84–86

Perhaps using FLANG_BINARY_DIR will fix the build?

[coti]

This comment was removed by coti.

[coti]

Followed @DavidTruby's recommendation on how include directories are set by CMake.

[sscalpone]

flang/tools/f18/CMakeLists.txt

30

Did this work for you?

CMake requires a PUBLIC or PRIVATE qualifier before the first directory argument.

I'm getting:

CMake Error at /local/home/sjs/llvm/llvm-project/flang/tools/f18/CMakeLists.txt:34 (target_include_directories):
  target_include_directories called with invalid arguments

[coti]

Actually, I cannot find a way to make it work with target_include_directories on a clean build tree.

With PRIVATE I get an error at the next step, what is being compiled is expecting an input on stdin:

[100%] Built target f18
[100%] Generating /__fortran_builtins.mod
Enter Fortran source
Use EOF character (^D) to end file

With PUBLIC

CMake Error in /home/users/coti/tmp/llvm-project/flang/tools/f18/CMakeLists.txt:
  Target "f18" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/home/users/coti/tmp/tmpbuild/tools/flang/include/flang"

  which is prefixed in the build directory.


CMake Error in /home/users/coti/tmp/llvm-project/flang/tools/f18/CMakeLists.txt:
  Target "f18" INTERFACE_INCLUDE_DIRECTORIES property contains path:

    "/home/users/coti/tmp/tmpbuild/tools/flang/tools/f18"

  which is prefixed in the build directory.

But, why not put f18_version.h in ${FLANG_BINARY_DIR}/include/flang? This way we only need to include this directory, and it is not totally irrelevant to have the version numbers in this directory, is it? However, I still cannot make target_include_directories work (same as the first error).

[sscalpone]

Did this work for you?

target_include_directories(f18
  PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
)

[coti]

On a clean build, with:

target_include_directories(f18
    PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
)

I get:

[ 91%] Building CXX object tools/flang/tools/f18/CMakeFiles/f18.dir/f18.cpp.o
/home/users/coti/tmp/llvm-project/flang/tools/f18/f18.cpp:40:10: fatal error: f18_version.h: No such file or directory
 #include "f18_version.h"
          ^~~~~~~~~~~~~~~
compilation terminated.

[sscalpone]

Sorry, I'm not sure what to do next. I'm using cmake vesion 3.18.0 and gcc 9.3.0.

Here's the full diff of my CMakefile:

diff --git a/flang/tools/f18/CMakeLists.txt b/flang/tools/f18/CMakeLists.txt
index 6103117123e..d7987e6341e 100644
--- a/flang/tools/f18/CMakeLists.txt
+++ b/flang/tools/f18/CMakeLists.txt
@@ -9,0 +10,3 @@ add_flang_tool(f18
+target_include_directories(f18
+  PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
+)
@@ -32,2 +34,0 @@ set(include ${FLANG_BINARY_DIR}/include/flang)
-set(include ${FLANG_BINARY_DIR}/include/flang)
-
@@ -82,0 +84 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/flang.sh.in ${FLANG_BINARY_DIR}/bin/f
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/f18_version.h.in ${CMAKE_CURRENT_BINARY_DIR}/f18_version.h @ONLY)

[richard.barton.arm]

Steve's patch works for me (cmake 3.17). @coti , your error message looks like what would happen if you replaced both definitions of {{include}} with the {{target_include_directories}} call. {{include}} is set up to the location of pre-built modules and is used in the compilation of said modules and then pre-processed into the flang.sh script. You still need that as well as setting {{target_include_directories}} to tell CMake where f18_version.h is when building f18.cpp.

Note for future (not part of this patch) those modules ought to end up in the lib directory of the install not the include directory. Clang puts similar headers in lib/clang/<version>/include and we should ultimately do the same for Flang. I think the include dir in the installation is for LLVM header files, e.g. if you wanted to use the LLVM libraries in some project or other.

[coti]

On a clean tree, with the attached patch, I get:

[ 90%] Built target f18
[ 90%] Generating /__fortran_builtins.mod
Enter Fortran source
Use EOF character (^D) to end file

And then it does not progress anymore...

[richard.barton.arm]

@coti the reason for the build failure is because you have removed both of the duplicate definitions of include in flang/tools/f18/CMakeLists.txt. This causes CMake not to be able to find the right files when it creates the builtin module files. One of these needs to be restored in your patch for the build to work as before.

I have marked the line in the original source that needs to be reinstated to your patch to make it work correctly for my in-tree build. If you re-submit a patch with that change then it can be tested out of tree.

flang/tools/f18/CMakeLists.txt

32

This is the line to reinstate.

[coti]

Fixed the include paths. Thank you @richard.barton.arm!

[richard.barton.arm]

This is working well for me in an in-tree build.

[richard.barton.arm]

I have tested this works well on an out of tree build (as it broke it before) so have no reservations about pushing the change again on @coti 's behalf.

[AlexisPerry]

This comment was removed by AlexisPerry.

[naromero77]

This comment was removed by naromero77.

flang/test/Driver/version_test.f90

 ! Check that lit configuration works by checking the compiler version
 
-! RUN: %f18 -V 2>&1 | FileCheck  -check-prefix=VERSION %s
 ! VERSION-NOT:{{![[:space:]]}}
 ! VERSION:{{[[:space:]]}}
-! VERSION-SAME:f18 compiler (under development)
+! VERSION-SAME:f18 compiler (under development), version {{[1-9][0-9]*.[0-9]*.[0-9]*}}
 ! VERSION-EMPTY:
+  
+! RUN: %f18 -V 2>&1 | FileCheck  -check-prefix=VERSION %s
+! RUN: %f18 -v 2>&1 | FileCheck  -check-prefix=VERSION %s
+! RUN: %f18 --version 2>&1 | FileCheck  -check-prefix=VERSION %s

flang/test/Preprocessing/compiler_defined_macros.F90

+! Check that the macros that give the version number are set properly
+
+!CHECK: flang_major = {{[1-9][0-9]*$}}
+!CHECK: flang_minor = {{[0-9]+$}}
+!CHECK: flang_patchlevel = {{[0-9]+$}}
+!RUN: %f18 -E %s | FileCheck  --ignore-case %s
+
+  
+integer, parameter :: flang_major = __flang_major__
+integer, parameter :: flang_minor = __flang_minor__
+integer, parameter :: flang_patchlevel = __flang_patchlevel__

flang/tools/f18/CMakeLists.txt

set(MODULES
   "ieee_exceptions"
   "ieee_features"
   "iso_c_binding"
   "iso_fortran_env"
   "omp_lib"
   "__fortran_builtins"
 )
 
 set(include ${FLANG_BINARY_DIR}/include/flang)
-
-set(include ${FLANG_BINARY_DIR}/include/flang)
+target_include_directories(f18
+  PRIVATE
+  ${CMAKE_CURRENT_BINARY_DIR}
+)
 
 # Create module files directly from the top-level module source directory
 foreach(filename ${MODULES})
   if(${filename} MATCHES "__fortran_builtins")
     set(depends "")
   else()
     set(depends ${include}/__fortran_builtins.mod)
   endif()
foreach(filename ${MODULES})
   install(FILES ${include}/${filename}.f18.mod DESTINATION include/flang)
 endforeach()
 
 add_custom_target(module_files ALL DEPENDS ${MODULE_FILES})
 
 install(TARGETS f18 DESTINATION bin)
 
 set(FLANG_INTRINSIC_MODULES_DIR ${FLANG_BINARY_DIR}/include/flang)
 
 # This flang shell script will only work in a POSIX shell.
 if (NOT WIN32)
   if (FLANG_STANDALONE_BUILD)
     configure_file(${CMAKE_CURRENT_SOURCE_DIR}/flang.sh.in ${CMAKE_BINARY_DIR}/tools/flang/bin/flang @ONLY)
     file(COPY ${CMAKE_BINARY_DIR}/tools/flang/bin/flang DESTINATION ${CMAKE_BINARY_DIR}/bin FILE_PERMISSIONS OWNER_EXECUTE OWNER_READ OWNER_WRITE)
   else()
     configure_file(${CMAKE_CURRENT_SOURCE_DIR}/flang.sh.in
       ${CMAKE_CURRENT_BINARY_DIR}/tools/flang/bin/flang @ONLY)
     add_custom_command(TARGET f18
         POST_BUILD
           COMMAND ${CMAKE_COMMAND} -E copy
           ${CMAKE_CURRENT_BINARY_DIR}/tools/flang/bin/flang
           ${LLVM_RUNTIME_OUTPUT_INTDIR}/flang
             COMMAND chmod +x ${LLVM_RUNTIME_OUTPUT_INTDIR}/flang)
   endif()
 endif()
 
 # The flang script to be installed needs a different path to the headers.
 set(FLANG_INTRINSIC_MODULES_DIR ${CMAKE_INSTALL_PREFIX}/include/flang)
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/flang.sh.in ${FLANG_BINARY_DIR}/bin/flang-install.sh @ONLY)
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/f18_version.h.in ${CMAKE_CURRENT_BINARY_DIR}/f18_version.h @ONLY)
 
 install(PROGRAMS ${FLANG_BINARY_DIR}/bin/flang-install.sh DESTINATION bin RENAME flang PERMISSIONS OWNER_EXECUTE OWNER_READ OWNER_WRITE)

flang/tools/f18/f18.cpp

 #include <fstream>
 #include <list>
 #include <memory>
 #include <optional>
 #include <stdlib.h>
 #include <string>
 #include <vector>
 
+#include "f18_version.h"
+
 static std::list<std::string> argList(int argc, char *const argv[]) {
   std::list<std::string> result;
   for (int j = 0; j < argc; ++j) {
     result.emplace_back(argv[j]);
   }
   return result;
 }
 
if (!driver.outputPath.empty()) {
     argv.push_back(driver.outputPath.data());
   }
   for (auto &lib : liblist) {
     argv.push_back(lib.data());
   }
   Exec(argv, driver.verbose);
 }
 
+int printVersion() {
+  llvm::errs() << "\nf18 compiler (under development), version "
+               << __FLANG_MAJOR__ << "." << __FLANG_MINOR__ << "."
+               << __FLANG_PATCHLEVEL__ << "\n";
+  return exitStatus;
+}
+
 int main(int argc, char *const argv[]) {
 
   atexit(CleanUpAtExit);
 
   DriverOptions driver;
   const char *F18_FC{getenv("F18_FC")};
   driver.F18_FCArgs.push_back(F18_FC ? F18_FC : "gfortran");
   bool isPGF90{driver.F18_FCArgs.back().rfind("pgf90") != std::string::npos};
 
   std::list<std::string> args{argList(argc, argv)};
   std::vector<std::string> objlist, liblist;
   std::string prefix{args.front()};
   args.pop_front();
   prefix += ": ";
   driver.prefix = prefix.data();
 
   Fortran::parser::Options options;
   options.predefinitions.emplace_back("__F18", "1");
   options.predefinitions.emplace_back("__F18_MAJOR__", "1");
   options.predefinitions.emplace_back("__F18_MINOR__", "1");
   options.predefinitions.emplace_back("__F18_PATCHLEVEL__", "1");
+  options.predefinitions.emplace_back("__flang__", __FLANG__);
+  options.predefinitions.emplace_back("__flang_major__", __FLANG_MAJOR__);
+  options.predefinitions.emplace_back("__flang_minor__", __FLANG_MINOR__);
+  options.predefinitions.emplace_back(
+      "__flang_patchlevel__", __FLANG_PATCHLEVEL__);
 #if __x86_64__
   options.predefinitions.emplace_back("__x86_64__", "1");
 #endif
 
   Fortran::common::IntrinsicTypeDefaultKinds defaultKinds;
 
   std::vector<std::string> fortranSources, otherSources;
   bool anyFiles{false};
 
   while (!args.empty()) {
     std::string arg{std::move(args.front())};
     auto dot{arg.rfind(".")};
     std::string suffix{arg.substr(dot + 1)};
     std::string prefix{arg.substr(0, 2)};
if (arg.empty()) {
           << "f18 options:\n"
           << "  -Mfixed | -Mfree | -ffixed-form | -ffree-form   force the "
              "source form\n"
           << "  -Mextend | -ffixed-line-length-132   132-column fixed form\n"
           << "  -f[no-]backslash     enable[disable] \\escapes in literals\n"
           << "  -M[no]backslash      disable[enable] \\escapes in literals\n"
           << "  -Mstandard           enable conformance warnings\n"
           << "  -std=<standard>      enable conformance warnings\n"
           << "  -fenable=<feature>   enable a language feature\n"
           << "  -fdisable=<feature>  disable a language feature\n"
           << "  -r8 | -fdefault-real-8 | -i8 | -fdefault-integer-8 | "
              "-fdefault-double-8   change default kinds of intrinsic types\n"
           << "  -Werror              treat warnings as errors\n"
           << "  -ed                  enable fixed form D lines\n"
           << "  -E                   prescan & preprocess only\n"
           << "  -module dir          module output directory (default .)\n"
           << "  -flatin              interpret source as Latin-1 (ISO 8859-1) "
if (arg.empty()) {
           << "  -fdebug-no-semantics  disable semantic checks\n"
           << "  -fget-definition\n"
           << "  -fget-symbols-sources\n"
           << "  -v -c -o -I -D -U    have their usual meanings\n"
           << "  -help                print this again\n"
           << "Unrecognised options are passed through to the external compiler\n"
           << "set by F18_FC (see defaults).\n";
       return exitStatus;
-    } else if (arg == "-V") {
-      llvm::errs() << "\nf18 compiler (under development)\n";
-      return exitStatus;
+    } else if (arg == "-V" || arg == "--version") {
+      return printVersion();
     } else {
       driver.F18_FCArgs.push_back(arg);
       if (arg == "-v") {
+        if (args.size() > 1) {
           driver.verbose = true;
+        } else {
+          return printVersion();
+        }
       } else if (arg == "-I") {
         driver.F18_FCArgs.push_back(args.front());
         driver.searchDirectories.push_back(args.front());
         args.pop_front();
       } else if (arg.substr(0, 2) == "-I") {
         driver.searchDirectories.push_back(arg.substr(2));
       }
     }

flang/tools/f18/f18_version.h.in

+#ifndef _F18_H_
+#define _F18_H_
+
+#define __FLANG__ "1"
+#define __FLANG_MAJOR__ "@LLVM_VERSION_MAJOR@"
+#define __FLANG_MINOR__ "@LLVM_VERSION_MINOR@"
+#define __FLANG_PATCHLEVEL__ "@LLVM_VERSION_PATCH@"
+
+#endif // _F18_H_