[llvm-dev] Plan for landing flang in monorepo

Tue Feb 25 09:38:36 PST 2020

Hi David,

This matches what I was seeing, thanks :)

-eric

On Tue, Feb 25, 2020 at 7:15 AM David Truby <David.Truby at arm.com> wrote:

> Hi Steve, Eric
>
> I believe (please correct me if I’m wrong!) that the only places that
> C-style string handling and manipulation currently happens is in the
> functions that interact with the file system and perform IO, as this is
> currently mostly written with posix IO calls. I am currently working
> actively on changing these cases to use LLVM’s file IO functions and/or
> llvm::MemoryBuffer, which is certainly necessary to happen before merging.
> As part of this, I will transition the string handling to use C++-style
> strings and manipulation. I’ve already done this for the module writer in
> this PR for example: https://github.com/flang-compiler/f18/pull/993
>
> Regardless of whether I’m right about that being the only place it
> happens, I think we should separate out two things here: replacing any
> C-style string handling with more modern C++ string handling, and replacing
> uses of std::string/std::string_view with llvm::SmallString/llvm::StringRef
> where appropriate. The former certainly should happen before the merge
> (again, regardless of whether it happens in more places than IO). What we
> meant in the list that Richard sent was that the latter is more open-ended
> and should happen after the merge.
>
> I hope that helps to clarify what we were talking about.
>
> David Truby
>
> On 25 Feb 2020, at 14:57, Steve Scalpone via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Hi Eric,
>
> Old flang certainly uses C-style strings but f18 uses std::string with few
> exceptions.  Most of the instances in f18 of “char *” aren’t really strings
> in the C sense – they’re not null terminated and are really just pointers
> into raw or cooked source files/streams.  I can’t think of an instance
> where the compiler dynamically allocates an array of characters and uses it
> as a C string.
>
> - Steve
>
>
> *From: *llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Eric
> Christopher via llvm-dev <llvm-dev at lists.llvm.org>
> *Reply-To: *Eric Christopher <echristo at gmail.com>
> *Date: *Monday, February 24, 2020 at 5:01 PM
> *To: *Timothy Keith <tkeith at nvidia.com>
> *Cc: *"llvm-dev at lists.llvm.org" <llvm-dev at lists.llvm.org>
> *Subject: *Re: [llvm-dev] Plan for landing flang in monorepo
>
> *External email: Use caution opening links or attachments*
>
> Last I looked there's a lot of char * based string manipulation in f18.
> I'd like that moved to std::string/string_view/StringRef uses.
>
> -eric
>
> On Mon, Feb 24, 2020 at 4:57 PM Timothy Keith <tkeith at nvidia.com> wrote:
>
> Can you elaborate on this?
> -  to move the std::string/string_view/StringRef changes to pre-merge
> unless you're going to have someone dedicated to handling them post-merge
> (rather than "time permits"). The C vs C++ ism here is fairly strong and
> I'd like to get the C-style string handling out fairly quickly.
>
> I understood this item to be looking into replacing uses of std::string
> with a more appropriate LLVM data structure where there is one. What is the
> “C-style string handling” part of it?
>
> Tim
>
> *From: *llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Eric
> Christopher via llvm-dev <llvm-dev at lists.llvm.org>
> *Reply-To: *Eric Christopher <echristo at gmail.com>
> *Date: *Monday, February 24, 2020 at 3:26 PM
> *To: *Richard Barton <Richard.Barton at arm.com>
> *Cc: *"llvm-dev at lists.llvm.org" <llvm-dev at lists.llvm.org>
> *Subject: *Re: [llvm-dev] Plan for landing flang in monorepo
>
> *External email: Use caution opening links or attachments*
>
> Hi Richard,
>
> Thanks for the work and mail with a plan. Other than the two comments
> below I think this plan is quite workable and am in support.
>
> I think the only requests I'd make is:
>  -  to move the std::string/string_view/StringRef changes to pre-merge
> unless you're going to have someone dedicated to handling them post-merge
> (rather than "time permits"). The C vs C++ ism here is fairly strong and
> I'd like to get the C-style string handling out fairly quickly. In my
> personal priority list I'd put this above the std:list migration.
>
>  - large scale renames: For naming issues I think you can automate a lot
> of it via clang-tidy ahead of time if you wanted to go down that path. I
> think it could turn it from a fairly arduous task to one that's a little
> easier.
>
> Thanks!
>
> -eric
>
> On Thu, Feb 20, 2020 at 9:41 AM Richard Barton <Richard.Barton at arm.com>
> wrote:
>
> Hi llvm-dev
>
> It's been a few weeks since I last gave an update on F18 and our progress
> on readying it for inclusion into the monorepo. Last time we discussed this
> the community challenged us to make the F18 source code look more like an
> LLVM project and to come up with a plan and schedule for completing this
> work (http://lists.llvm.org/pipermail/llvm-dev/2020-January/137989.html)
>
> The full list of changes that could be made to make F18 more LLVM-like is
> very long. We're interested in identifying what the absolute dealbreakers
> are that block inclusion into the monorepo and which changes would be
> acceptable to make after inclusion to the monorepo. We've come up with
> strawman lists for each and would like to propose the following plan of
> action:
> 1.      We have captured our strawman proposal for all the changes that
> need to happen to F18 to make it ready for inclusion into the monorepo on a
> github project board: https://github.com/orgs/flang-compiler/projects/8 (also
> repeated at the end of this mail.)
> 1.      We are working through this list and we believe that we can
> complete this work in time for a new upstreaming date of 16th March.
> 2.      We have captured further work that we plan to complete on F18
> after merging to the monorepo
> https://github.com/orgs/flang-compiler/projects/10(also repeated below)
> 3.      We believe that we can complete this work before the LLVM11
> branching date in June.
> 4.      After this date, we'll keep improving the code as we go along and
> not on any specific timescale.
>
> We'd really appreciate feedback on the two lists of changes, specifically:
> are these lists complete? Is everyone satisfied that with all the items on
> https://github.com/orgs/flang-compiler/projects/8, we'd be happy to
> accept F18 into the monorepo? Are there any further changes that would need
> to be made to F18 for this to happen?
>
> Thanks
> Rich
>
> More info on the lists:
>
> Pre-merge list: https://github.com/orgs/flang-compiler/projects/8
>
> The status today is that many of the items on the pre-merge list are well
> underway or complete.
>
> 1.      Integrate into the monorepo CMake
> a.       This will be as an optional project, and default to not building.
> b.      This also adds Doxygen infrastructure so we can start to improve
> interface documentation and continue post-merge.
> 2.      F18 changes to make it more LLVM-like in code style
> c.       Rationalise headers to put public headers in /include and not
> /lib
> d.      Examine F18's clang-format file and minimise deviations to the
> LLVM clang-format
> e.       Rename all .cc files to .cpp
> f.       Capitalize the module directory names in /lib and /include (e.g.
> /lib/Parser)
> 3.      Increase use of LLVM APIs and utilities in F18
> g.       Switch F18 custom File handling to LLVM's File handling (helps
> with non-POSIX platform support)
> h.      Change uses of C++ standard stream IO library to LLVM's
> equivalent library
> i.        Audit use of std::list and consider migrating to a suitable
> alternative in LLVM's API
> j.        Use llvm_unreachable with an error message for unreachable cases
>
> 4.      Convert the regression test suite to using lit rather than ctest
> k.       Porting off the custom scripts to FileCheck will continue after
> this but we think it should not gate inclusion to the monorepo.
> 5.      Ensure that F18 builds with the same compilers as the rest of the
> monorepo
> l.        One caveat is that we can only support C++17 compilers
> m.    We propose to defer Windows support until after we merge
> n.      We will specifically also check with the latest LLVM 10 rc
>
> Post-merge list: https://github.com/orgs/flang-compiler/projects/10
>
> This is the work that will happen right away after merging to the monorepo
>
> 1.      F18 changes to make it more LLVM-like in code style - We will
> perform a one-off exercise where we audit the code to find these instances
> and bring them in line. We'll look at:
> a.       Braces on all single-line if statements
> b.      Uses of else-after-return.
> 2.      Increase use of LLVM APIs and utilities in F18 - We'll audit the
> uses of these datatypes and try to move to a suitable LLVM alternative
> c.       std::string/std::string_view
> d.      std::vector
> e.       std::set
> f.       std::map
> 3.      Further work on porting the test suite to make it more LLVM-like
> g.       Port lit tests to FileCheck
> h.      Port unit tests to gtest
> i.        Implement equivalent to clang -verify and port tests to that
> 4.      Support Windows
> j.        Porting to LLVM file I/O is the main blocker - included in the
> post-merge worklist - but there will be more to do after this.
> k.       Isuru Fernando is going to lead this effort
> 5.      Set up official builders
> l.        Arm will handle bots for AArch64
> m.    Nvidia will handle X86
> n.      Tarique Islam at IBM will set up a builder for Power:
> http://lists.llvm.org/pipermail/flang-dev/2020-February/000232.html
> o.      Any further help from community bot maintainers to cover all the
> platforms and compilers would be greatly appreciated.
>
> One specific ask in the last round of feedback was on sharing
> lib/common/Although we see the benefit of doing this exercise, we feel it
> is a bit too early to start. One design principle we wish to stick to is
> for the Fortran runtime and compiler align on their implementations. For
> the specific example of
> https://github.com/flang-compiler/f18/blob/master/include/flang/common/bit-population-count.h
>  we'd want the compiler and runtime POPCNT intrinsic to align on
> implementation. The F18 runtime is still a work in progress. We need to
> decide on how or if this could share code with LLVM libraries and then we
> can revisit the implementations in include/flang/common.
>
> Future work
> After all the above is done, we will continue to bring the code more in
> line with LLVM style and API usage by fixing things as we find them during
> development and enforce the new style through code review. A few specific
> areas that have been mentioned before that we will tackle in this way are:
>
> 1.      Add Doxygen style comments to interfaces
> 2.      Classes, files, names, etc. where a more LLVM-standard naming can
> be used.
> 3.      Refactor code to use early exits when suitable
> 4.      Audit functions in include/flang/common and port to LLVM
> equivalents (e.g. builtin_popcount)
>
>
>
> ------------------------------
> This email message is for the sole use of the intended recipient(s) and
> may contain confidential information.  Any unauthorized review, use,
> disclosure or distribution is prohibited.  If you are not the intended
> recipient, please contact the sender by reply email and destroy all copies
> of the original message.
> ------------------------------
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200225/79c22697/attachment.html>