This is an archive of the discontinued LLVM Phabricator instance.

[Flang] Add a link from the docs html page to the FIR html page
ClosedPublic

Authored by DylanFleming-arm on Jun 27 2022, 7:44 AM.

Details

Summary

The Fortran Language Reference is currently generated via tablegen,
however isn't present on flang.llvm.org/docs/

This patch adds FIRLangRef.md to the flang/docs directoy,
and adds a link to the generated HTML file in sidebar
under the 'Documentation' heading.

Diff Detail

Event Timeline

Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
DylanFleming-arm requested review of this revision.Jun 27 2022, 7:44 AM

The Fortran Language Reference is currently generated via tablegen, however isn't present on flang.llvm.org/docs/

Nit: Fortran IR (FIR) Language Reference

If the rendering for the first entry is not yet fixed, please call out that in the summary.

flang/docs/CMakeLists.txt
105

-> If we copy this into the top-level directory, will that automatically create an entry on the main page, besides the sidebar?
-> I was thinking we will be copying the *.md files in flang/docs somewhere in the Binary Directory and then we will build flang-doc target and then we will copy the FIRLangRef.md file to a sub-directory in the Binary directory. Usually copying generated files in the binary directory to the source directory is a good idea or not.

kiranchandramohan requested changes to this revision.Jun 28 2022, 3:04 AM
kiranchandramohan added inline comments.
flang/docs/CMakeLists.txt
105

I meant, that copying generated files to the source directory is generally not a good idea.

This revision now requires changes to proceed.Jun 28 2022, 3:04 AM
awarzynski added inline comments.Jun 28 2022, 3:21 AM
flang/docs/CMakeLists.txt
105

I agree, the source directory should not be modified during the CMake configuration or the build process.

Changed summary to include problem with incorrect formatting.
Removed copying of FIRLangRef to source.

You are making a change similar to https://reviews.llvm.org/D72875 - I would refer to that Clang patch (it explains, amongst other things, the requirement for copying things across). Could you also expand the comments to explain the need for copying files?

flang/docs/CMakeLists.txt
98

Is /Source at the end required?

Expanded comments around copying files.

The /Source is required, it defines the director where conf.py is located, it'll error without it.
While creating the directory itself isn't 100% necessary, there's multiple files/subdirectories
being copied so I think it's just cleaner to have them all in one directory, even if it does require that '/Source'

Do we still have the requirement to build this twice? I got an error while clicking "FIR Language Reference". If it is solved by the python script, then is it better to merge D129186 into this?

Ah, I must've forgotten about that problem,

I've combined the copying of FIRLangRef.md and the src files into one custom target, which should now mean they both happen before html is generated.
That should fix the problem!

flang/docs/CMakeLists.txt
100–101

Is there any guarantee on the order in which these dependencies are run?

114

What is the guarantee that at this point FIRLangRef.md is already generated?

flang/docs/CMakeLists.txt
101

Does the following make sense?

Change

add_dependencies(docs-flang-html flang-doc)

to

add_dependencies(copy-flang-src-docs flang-doc)

and move it after add_custom_target(copy-flang-src-docs.

I've moved the dependency inside the custom_target declaration, it should resolve the issue of dependency ordering.

LGTM. Have a Nit comment. Please address and submit.

flang/docs/CMakeLists.txt
105

Nit: Please rewrite this in a suitable manner. A suggestion given below.

Copy the flang/docs directory and the generated FIRLangRef.md file to a place in the binary directory. Having all the files in a single directory makes it possible for Sphinx to process them together. Add a dependency to the flang-doc target to ensure that the FIRLangRef.md file is generated before the copying happens.

106

Nit: to the binary directory.

This revision is now accepted and ready to land.Jul 11 2022, 8:15 AM