[PATCH] D134821: [flang][driver] Allow main program to be in an archive

Thu Sep 29 13:55:24 PDT 2022

awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

Great stuff @ekieri , thanks for doing this!

You may want add a note in the summary that you've updated the docs as well.  This is consistent with what we discussed in https://github.com/llvm/llvm-project/issues/54787. I've left a few minor comments, but otherwise LGTM.

================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:604-606
+    // A Fortran main program will be lowered to a function named _QQmain. Make
+    // _QQmain an undefined symbol, so that we include it even if it hides
+    // inside an archive.
----------------
I would add a reference to the GitHub issue too.

================
Comment at: flang/docs/FlangDriver.md:152-158
-Note that currently Flang does not support code-generation and `flang-new` will
-fail during the second step above with the following error:
-```bash
-error: code-generation is not available yet
-```
-The other phases are printed nonetheless when using `-ccc-print-phases`, as
-that reflects what `clangDriver`, the library, will try to create and run.
----------------
Oh, good catch, completely forgot about this!

This is an unrelated change, but the noise is low and I'm happy for it to be included. You can also submit it as a separate patch if you want.

================
Comment at: flang/docs/FlangDriver.md:202
+## Linker invocation
+Note: Linker invocation through the flang-new driver is so far
+experimental. This section describes the currently intended design, and not
----------------
awarzynski wrote:
> I used similar syntax in https://github.com/llvm/llvm-project/blob/release/14.x/flang/docs/FlangDriver.md. WDYT?

================
Comment at: flang/docs/FlangDriver.md:202-204
+Note: Linker invocation through the flang-new driver is so far
+experimental. This section describes the currently intended design, and not
+necessarily what is implemented.
----------------
I used similar syntax in https://github.com/llvm/llvm-project/blob/release/14.x/flang/docs/FlangDriver.md. WDYT?

================
Comment at: flang/docs/FlangDriver.md:208
+```bash
+flang-new prog.f90
+```
----------------
;-)

================
Comment at: flang/docs/FlangDriver.md:211
+will, on a high level, do two things:
+* call the frontend driver to build the object file prog.o, and
+* call the system linker to build the executable a.out.
----------------

================
Comment at: flang/docs/FlangDriver.md:214
+
+In both invocations, flang-new will add default options. To see the exact
+invocations on your system, you can call
----------------
> add default options

Perhaps clarify "where" (i.e. to the frontend driver and the linker invocations).

================
Comment at: flang/docs/FlangDriver.md:220
+The link line will contain a fair number of options, which will depend on your
+system. Compared to when linking a c program with clang, the main additions are
+(on GNU/linux),
----------------

================
Comment at: flang/docs/FlangDriver.md:223
+* `--undefined=_QQmain`: A fortran main program will appear in the corresponding
+  object file as a function called `_QQmain`. This flag lets an object file
+  containing a fortran main program (i.e., a symbol `_QQmain`) be included in
----------------

================
Comment at: flang/docs/FlangDriver.md:224
+  object file as a function called `_QQmain`. This flag lets an object file
+  containing a fortran main program (i.e., a symbol `_QQmain`) be included in
+  the linking also when it is bundled in an archive.
----------------

================
Comment at: flang/docs/FlangDriver.md:226
+  the linking also when it is bundled in an archive.
+* `-lFortran_main`: The Fortran_main archive is part of flang's runtime. It
+  exports the symbol `main`, i.e., a c main function, which will make some
----------------

================
Comment at: flang/docs/FlangDriver.md:229
+  initial configuration before calling `_QQmain`, and clean up before returning.
+* `-lFortranRuntime`: Flang's fortran runtime, containing, for example,
+  implementations of intrinsic functions.
----------------

================
Comment at: flang/docs/FlangDriver.md:231
+  implementations of intrinsic functions.
+* `-lFortranDecimal`: Part of flang's runtime, containing routines for parsing
+  and formatting decimal numbers.
----------------

================
Comment at: flang/docs/FlangDriver.md:233
+  and formatting decimal numbers.
+* `-lm`: Link with the math library, on which flang's runtime depends.
+
----------------

================
Comment at: flang/test/Driver/link-c-main.c:7
+
+REQUIRES: x86-registered-target, system-linux, c-compiler
+
----------------
Same for the other test. Let me know if you'd like somebody to test this for you on AArch64 ;-)

================
Comment at: flang/test/Driver/link-c-main.c:25-26
+/*
+CHECK-DAG: F .text {{[a-f0-9]+}} main
+CHECK-DAG: *UND* {{[a-f0-9]+}} _QQmain
+*/
----------------
I'd be tempted to add a note that:

* `main` ought to be defined as it's specified in this file
* `_QQmain` should remain undefined as no Fortran-based object file generated by Flang is linked in

This is quite tricky stuff and in such cases "more is more" :)

================
Comment at: flang/test/Driver/linker-flags.f90:54-58
+! Linker invocation to generate a shared object
+! GNUSO-LABEL:  "{{.*}}ld"
+! GNUSO-SAME: "[[object_file]]"
+! GNUSO-SAME: -lFortranRuntime
+! GNUSO-SAME: -lFortranDecimal
----------------
How is this different to `GNU-SAME`? 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134821/new/

https://reviews.llvm.org/D134821