pscoro added inline comments.

================
Comment at: flang/runtime/CMakeLists.txt:251
 
-  INSTALL_WITH_TOOLCHAIN
-)
+if (DEFINED LLVM_ENABLE_RUNTIMES AND "flang-rt" IN_LIST LLVM_ENABLE_RUNTIMES)
+  add_flang_library(FortranRuntime STATIC
----------------
efriedma wrote:
> pscoro wrote:
> > efriedma wrote:
> > > This "if" doesn't make sense to me.  If we're not building flang-rt, we 
> > > shouldn't be here, so I don't see why you need an "if" in the first place.
> > `add_subdirectory(runtime)` is a line that still exists in 
> > `flang/CMakeLists.txt`. This exists because `Fortran_main` is still being 
> > built at the same time as the compiler, and to do so, the runtime 
> > subdirectory still needs to be added to flang (`flang/CMakeLists.txt` -> 
> > `add_subdirectory(runtime)` -> `flang/runtime/CMakeLists.txt` -> 
> > `add_subdirectory(FortranMain)`. The solution I had was to just add a check 
> > around the `FortranRuntime` library production so that it only happens for 
> > flang-rt.
> > 
> > If you have a better solution let me know. Overall, I'm not sure if 
> > Fortran_main is currently being handled in the best way (ie, its still 
> > being built at the same time as the compiler, which doesn't seem ideal), 
> > but am not sure what course of action to take with it since it doesn't 
> > really belong in flang-rt either (see documentation for details)
> Fortran_main should be "part of" flang-rt in the sense that building flang-rt 
> builds it.  Most of the same reasons we want to build flang-rt.a as a runtime 
> apply.
> 
> Since the output needs to be separate, building flang-rt should produce two 
> libraries: flang-rt.a and FortranMain.a.
I agree with this idea and have been trying to make it work but in the process 
I discovered that my "fix" above (`if (DEFINED LLVM_ENABLE_RUNTIMES AND 
"flang-rt" IN_LIST LLVM_ENABLE_RUNTIMES)`) is worse than I thought.

By building the llvm target with flang-rt as an enabled runtime, we are 
essentially saying: build the flang compiler, and then invoke cmake again to 
build the runtimes project (externally), which builds flang-rt.

The problem is that check-all depends on check-flang which depends on the 
runtime. The if guard above was not actually doing what I thought it was, and 
the reason why configuring didnt fail with "flang-rt does not exist" is because 
the if guard was still evaluating to true. Basically, the guard ensured that 
FortranRuntime was only being built if flang-rt is built, but the add_library 
call was still being reached *during the configuration of the flang compiler*.

I am having trouble dealing with this dependency since runtimes get built 
externally (and after the compiler is built, which is when the check-all custom 
target gets built). I will have to investigate more closely how other runtimes 
handle this. My initial thought was perhaps the runtime testing should be 
decoupled from check-flang/check-all however I don't know if that's a good 
idea, and I also think that only helps with flang-rt, but if Fortran_main is 
required to build any executable I imagine that it should be needed for 
check-flang?

This is just an update of whats holding me back right now. If you have any tips 
please let me know. Thanks for bringing this to my attention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154869

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to