llvm dev - Jan 2020 - Flang landing in the monorepo - next Monday!

Hi Eric, Renato

Thanks again for the engagement and challenge on this, it is really useful
feedback to know what we need to do to get F18 into the project in a way that
everyone is happy with.

I have tried to give timelines on the points addressed below where I can today.
Clearly we need to do some work on points 8-11, but are the above plans/answers
to points 1-7 sufficient at this stage and would not block a merge of F18 to the
monorepo? If not, what else would you need to see or what would need to be
different for you to be happy with F18 merging into the monorepo?
>> 4. No use of LLVM APIs and so no connection to the project [Addressed
by Hal and me - it is the natural next step in development as Flang starts to
generate MLIR. Nvidia are working on this now.]
> The choice of MLIR is interesting - and given that there was no discussion
of it I was wondering if you have a document laying out pros/cons and what
ultimately made the decision here.
Hopefully Steve’s reply has covered your concerns about the design choice. Also
to point out that there has been a lot of development done on this approach
already, which Eric has shared [1] (still a work in progress), and this has
reinforced our feeling that a) having a high-level IR above LLVM IR is a good
idea and b) using MLIR for that is a good idea.

The next step for this work is to break it down into a logical stream of patches
and go through review to get them committed. Our preference would be to do this
in phabricator in the llvm project after merging. We think the benefit of doing
this review in the llvm project is that more reviewers can participate,
including those from the MLIR project. We also hope that by developing FIR
in-tree we can more easily manage its MLIR dependency.

I don’t think we can be sure of a timeline for completion of this work. In
reality adding this part of the frontend will likely be an ongoing effort over
the next months, perhaps years and it will likely be a work in progress by the
time of the LLVM11 branch. We can at least commit to starting this effort soon,
lead by Nvidia. If we don't merge to monorepo shortly then the work will
start on the F18 github project. Arm engineers will be reviewing, but we think
the benefit of doing this development in the monorepo is that other interested
parties can join in from the MLIR and LLVM projects. We'd ask that we merge
to the monorepo before the review starts.
 
> 5. No use of lit in the regression tests [Arm is working on this]
This effort has started already in the F18 project [2]. Arm have some work
downstream to make the next steps – porting the old tests over to use lit.
Meanwhile, we would expect make check-all to run all the new lit tests as well
as the old non-lit tests until the latter are all ported over to lit. We expect
to have this completed before the LLVM11 branch. We propose that this not gate
merge to the monorepo and we switch development from github to phab as soon as
the merge happens.
> 6. Need to refactor parts of clang driver that can be shared with flang
into a separate library [Arm is working on this, but plans to implement a simple
driver first before refactoring to better understand the opportunities to do so.
See Peter's RFC [d] ]
Similar to above, this effort has started. Arm’s next step after contributing
the flang entry point is to agree on a spec for the replacement frontend driver.
Given our expectation as to what this will contain, we expect to be able to
complete before the LLVM11 branch. We propose this not block merging to the
monorepo and we switch development from github to phab as soon as the merge
happens. In terms of refactoring the code to share with Clang, our proposal is
to implement the Flang side work then refactor later on once we understand the
code better. We would be very happy to reconsider this and put up another RFC if
we want to refactor as we go along instead. This approach is much easier once
F18 is accepted into the monorepo so we can make the changes together.
> > 7. No integration into the LLVM build system/Cmake 
> I don't really consider this addressed without a plan of action (before
merge) that contains a list of standard llvm support libraries that would be
used in addition to things being removed (pretty much all use of C++ c* headers
and more?) as well as staffing and approximate ETAs.
Eric Schweitz has implemented this as part of his patch that adds FIR [1]. Our
plan is to extract this as a separate patch in phabricator for review so it can
land more or less right away after F18 is merged into the monorepo. To be clear,
our intention is that flang would *not* build by default after the patch.
>> 8. No use of LLVM utilities or standard data structures
>> 9. Simple deviations from the LLVM coding style
>>     a. Separating public headers into include/flang
>>     b. Syntactical things like braces on single line statements,
comments on end of namespaces, etc.
>>     c. .cc file extensions rather than .cpp
>> 10. Bigger deviations from the LLVM coding style that are harder to fix
>>     a. Early exits and not using else after return, etc.
>> 11. Flang not supporting all the same C++ compilers as the rest of the
project (even taking into account C++17 requirement)
> As well as this. A plan of action isn't just a "we'll get to
this" but an actual plan :)
We will discuss these points on the flang-dev list now and report back with our
plans. We'll work on a list of items to fix before merge and list that we
propose to fix after merge on a shorter timescale. I welcome you to respond on
those threads if there are some specific recommendations or key issues you care
about. In any case, we'll reply back with our proposal.

@Renato - I just wanted to answer a few points from your reply to make sure
there is no misunderstanding here about F18.
> Now, I am surprised it's not using LLVM API yet (a clear and
> consistent complaint of the previous one), as well as having its own
> driver, test infra, etc.
The previous flang (to be absolutely clear - I mean this one:
https://github.com/flang-compiler/flang) did not use the LLVM API as you say,
instead writing a .ll file and calling clang on that file. F18 will certainly
use LLVM APIs to generate IR directly but that code has not made it to the
master F18 branch today. It is not true to say that F18 has inherited this flaw
from its predecessor.
> The lack of C++ compatibility may look small now, but once we want to
> move to a newer standard, Flang will be the bottleneck. In a monorepo,
> these things are even more important.
> F18 was supposed to be a whole new front-end, from scratch, but it
> looks to have carried along many of the previous projects' flaws.
To clarify, F18 code is compatible to C++17 so it is currently ahead of the
standard used by the rest of the LLVM project. That is an issue by itself that
needs managing, but a different problem from the one you describe here. The
problem with F18 being ahead of the rest of the project was discussed at length
during the original upstreaming discussion
http://lists.llvm.org/pipermail/llvm-dev/2019-February/130497.html and we agreed
to revisit the problem when F18 is part of the project and going more mainstream
and see where the rest of the project was in terms of its minimum standard. At
that point we can chose what to do.

Thanks again for caring about this and your help.

Ta
Rich

[1] https://github.com/flang-compiler/f18/pull/920
[2]
https://github.com/flang-compiler/f18/commit/a58c6067a1ab5cd02dfb5b6fb9919a20b960d984
[3] https://reviews.llvm.org/D63607,
https://github.com/flang-compiler/f18/pull/759,
https://github.com/flang-compiler/f18/pull/762 and
https://github.com/flang-compiler/f18/pull/763


From: Eric Christopher <echristo at gmail.com> 
Sent: 13 January, 2020 06:51
To: Richard Barton <Richard.Barton at arm.com>
Cc: Renato Golin <rengolin at gmail.com>; Hans Wennborg <hans at
chromium.org>; llvm-dev at lists.llvm.org; Mike Edwards <medwards at
llvm.org>
Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!



On Thu, Jan 9, 2020 at 4:28 AM Richard Barton via llvm-dev <mailto:llvm-dev
at lists.llvm.org> wrote:
Hi all

Thanks for all the replies and engagement on this issue.

First point, given the state of discussions today I would like to propose that
we don't start the merge at 10:00 GMT on Monday 13th as proposed and we
delay by at least 24 hours until after the scheduled F18 technical call on
Monday afternoon.

In order to help compile a plan of action, I've tried to compile a list of
the concerns that folks have raised so far.

I think all these have been addressed (please correct me if you think otherwise)
1. Audit trail/visibility of code review [Addressed by Peter - code has been
reviewed [a] to F18 coding guidelines [b].
2. Long-term viability of Flang community and overlap with existing LLVM
community [Hopefully Hal and Johannes replies and Greg's and Pat's and
my reply demonstrate long-term commitment to Flang after upstreaming]
3. Compatibility of license [Addressed by Steve, a recent update has made the
licenses compatible [c]]
4. No use of LLVM APIs and so no connection to the project [Addressed by Hal and
me - it is the natural next step in development as Flang starts to generate
MLIR. Nvidia are working on this now.]

The choice of MLIR is interesting - and given that there was no discussion of it
I was wondering if you have a document laying out pros/cons and what ultimately
made the decision here.
 
I think these are acknowledged right now and we are actively working on fixes:
5. No use of lit in the regression tests [Arm is working on this]
6. Need to refactor parts of clang driver that can be shared with flang into a
separate library [Arm is working on this, but plans to implement a simple driver
first before refactoring to better understand the opportunities to do so. See
Peter's RFC [d] ]
7. No use of LLVM utilities or standard data structures (Vector, etc.) [I think
Pat and Eric have patches ready to go for this?]

I don't really consider this addressed without a plan of action (before
merge) that contains a list of standard llvm support libraries that would be
used in addition to things being removed (pretty much all use of C++ c* headers
and more?) as well as staffing and approximate ETAs.
 

I think these are only acknowledged, with the intention to remediate post merge,
but no concrete plan or owner at this point:
8. Simple deviations from the LLVM coding style
    a. Separating public headers into include/flang
    b. Syntactical things like braces on single line statements, comments on end
of namespaces, etc.
    c. .cc file extensions rather than .cpp
9. Bigger deviations from the LLVM coding style that are harder to fix
    a. Early exits and not using else after return, etc.
10. Flang not supporting all the same C++ compilers as the rest of the project
(even taking into account C++17 requirement) 
As things stand we are not proposing to remediate these (issues 5 and greater)
in the code before it is merged to LLVM next week. These issues will be worked
on after commit.

As well as this. A plan of action isn't just a "we'll get to
this" but an actual plan :)
 
Does anyone feel passionately that any of the above points need to be addressed
in the code before we can merge?
Does anyone feel there is anything missing from that list?

Given Mehdi and Hans' query about why this needs to go into LLVM10. Unless
anyone has any good reason, perhaps we could postpone until after the branch,
perhaps by one week to January 20th? That would give us some more time to bottom
out what changes need to be made before we can accept the code and give us time
to make any changes that block inclusion (assuming they are suitably sized to do
in this timescale). As Renato suggests, that would also give us the LLVM11
branching point as a natural deadline to have addressed the larger concerns in
our list.

What do people think to that proposal?

I think waiting until after the llvm 10 branch would probably be best at this
point. That will give a lot of time for cleanup and to make f18 a much more
reasonable "preview" including code generation in a forthcoming
release in about 6 mos.

-eric
 
Regards
Rich

[a] https://github.com/flang-compiler/f18/pulls
[b]
https://github.com/flang-compiler/f18/blob/master/documentation/C%2B%2Bstyle.md
[c]
https://github.com/flang-compiler/f18/commit/e06be2faa64a52471b3cfb2829dc05888236aa68
[d]  http://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html
> -----Original Message-----
> From: llvm-dev <mailto:llvm-dev-bounces at lists.llvm.org> On Behalf
Of Renato
> Golin via llvm-dev
> Sent: 9 January, 2020 10:02
> To: Hans Wennborg <mailto:hans at chromium.org>
> Cc: llvm-dev <mailto:llvm-dev at lists.llvm.org>; Mike Edwards
<mailto:medwards at llvm.org>
> Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!
> 
> On Thu, 9 Jan 2020 at 08:13, Hans Wennborg via llvm-dev
> <mailto:llvm-dev at lists.llvm.org> wrote:
> > > Why is it targeting pre-llvm10 branching? Are we expecting to
ship
> anything in LLVM 10? If so I would have expected it to land much much
> earlier to be able to stabilize during the development phase long before
the
> branching point.
> >
> > I was wondering about this too. I'm not necessarily opposed, but
> > wouldn't it make sense to target landing soon *after* the branch
> > instead, to give it time to integrate, stabilize, and then ship in the
> > next release?
> 
> Usually, yes.
> 
> I'm guessing this is due to downstream implementations wanting to pick
> up the current release as base target (which can be done regardless,
> but...).
> 
> In theory we can hide flang by having to explicitly name it on the
> build command, so merging now or later won't make massive practical
> change.
> 
> I personally don't like rushing things like that, but I'm not
strongly
> opposed to either, if everyone else is on board with the rush.
> 
> cheers,
> --renato
> _______________________________________________
> LLVM Developers mailing list
> mailto:llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
mailto:llvm-dev at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Hi Rich,

On Wed, 15 Jan 2020 at 09:11, Richard Barton <Richard.Barton at arm.com>
wrote:
> I don’t think we can be sure of a timeline for completion of this work. In
reality adding this part of the frontend will likely be an ongoing effort over
the next months, perhaps years and it will likely be a work in progress by the
time of the LLVM11 branch. We can at least commit to starting this effort soon,
lead by Nvidia. If we don't merge to monorepo shortly then the work will
start on the F18 github project. Arm engineers will be reviewing, but we think
the benefit of doing this development in the monorepo is that other interested
parties can join in from the MLIR and LLVM projects. We'd ask that we merge
to the monorepo before the review starts.
I wouldn't like to forcefuly tie in LLVM API support in Flang with
MLIR-LLVM dialect support, as the latter is good, but not necessarily
complete enough.

Ideally, it would be best to tackle them at the same time (no point in
doing two complex projects if the latter replaces the former), but I'm
not sure LLVM will be ready to accept MLIR as an input IR off the bat.

I have no opinion on either, I just want to make sure we're taking an
informed decision here.

Doing both at the same time may push the the work past the 11 branch,
which then will mean 10, 11 and 12 will have three different stages
with their own incompatibilities.

This sucks really bad for users.

> The previous flang (to be absolutely clear - I mean this one:
https://github.com/flang-compiler/flang) did not use the LLVM API as you say,
instead writing a .ll file and calling clang on that file. F18 will certainly
use LLVM APIs to generate IR directly but that code has not made it to the
master F18 branch today. It is not true to say that F18 has inherited this flaw
from its predecessor.
I didn't mean code inheritance, just design inheritance.

My point was that there were clear feedback from last time, mainly:
use LLVM API, use LLVM testing infrastructure, use LLVM coding
standard.

However, despite being (I was told) a completely new project, those
major flaws still exists and they are, again, getting in the way of
merging Flang.

It feels as if the message wasn't clear enough.

> To clarify, F18 code is compatible to C++17 so it is currently ahead of the
standard used by the rest of the LLVM project. That is an issue by itself that
needs managing, but a different problem from the one you describe here.
I stand corrected, thanks! :)

cheers,
--renato

Hi Richard,

On Wed, Jan 15, 2020 at 1:11 AM Richard Barton <Richard.Barton at arm.com>
wrote:
> Hi Eric, Renato
>
> Thanks again for the engagement and challenge on this, it is really useful
> feedback to know what we need to do to get F18 into the project in a way
> that everyone is happy with.
>
> I have tried to give timelines on the points addressed below where I can
> today. Clearly we need to do some work on points 8-11, but are the above
> plans/answers to points 1-7 sufficient at this stage and would not block a
> merge of F18 to the monorepo? If not, what else would you need to see or
> what would need to be different for you to be happy with F18 merging into
> the monorepo?
>
> >> 4. No use of LLVM APIs and so no connection to the project
[Addressed
> by Hal and me - it is the natural next step in development as Flang starts
> to generate MLIR. Nvidia are working on this now.]
> > The choice of MLIR is interesting - and given that there was no
> discussion of it I was wondering if you have a document laying out
> pros/cons and what ultimately made the decision here.
>
> Hopefully Steve’s reply has covered your concerns about the design choice.
> Also to point out that there has been a lot of development done on this
> approach already, which Eric has shared [1] (still a work in progress), and
> this has reinforced our feeling that a) having a high-level IR above LLVM
> IR is a good idea and b) using MLIR for that is a good idea.
>
>
Yes. That was a good post and while I haven't gotten a chance to read
everything yet I appreciate the post.

FWIW my concerns are more than just "uses llvm to generate code" and
as
I've outlined a few times there are a lot of specific things that you
should be using LLVM for and C like C++ is another. I did a quick look over
the code and summarized a few of the examples in an earlier post. Please
try to avoid conflating "using llvm" with "using llvm to emit IR
of some
sort".

I think that affects a lot of the "we can't give a timeline" in
your email.
Can you comment further on the other code issues I've raised?

-eric

> The next step for this work is to break it down into a logical stream of
> patches and go through review to get them committed. Our preference would
> be to do this in phabricator in the llvm project after merging. We think
> the benefit of doing this review in the llvm project is that more reviewers
> can participate, including those from the MLIR project. We also hope that
> by developing FIR in-tree we can more easily manage its MLIR dependency.
>
> I don’t think we can be sure of a timeline for completion of this work. In
> reality adding this part of the frontend will likely be an ongoing effort
> over the next months, perhaps years and it will likely be a work in
> progress by the time of the LLVM11 branch. We can at least commit to
> starting this effort soon, lead by Nvidia. If we don't merge to
monorepo
> shortly then the work will start on the F18 github project. Arm engineers
> will be reviewing, but we think the benefit of doing this development in
> the monorepo is that other interested parties can join in from the MLIR and
> LLVM projects. We'd ask that we merge to the monorepo before the review
> starts.
>
> > 5. No use of lit in the regression tests [Arm is working on this]
> This effort has started already in the F18 project [2]. Arm have some work
> downstream to make the next steps – porting the old tests over to use lit.
> Meanwhile, we would expect make check-all to run all the new lit tests as
> well as the old non-lit tests until the latter are all ported over to lit.
> We expect to have this completed before the LLVM11 branch. We propose that
> this not gate merge to the monorepo and we switch development from github
> to phab as soon as the merge happens.
>
> > 6. Need to refactor parts of clang driver that can be shared with
flang
> into a separate library [Arm is working on this, but plans to implement a
> simple driver first before refactoring to better understand the
> opportunities to do so. See Peter's RFC [d] ]
> Similar to above, this effort has started. Arm’s next step after
> contributing the flang entry point is to agree on a spec for the
> replacement frontend driver. Given our expectation as to what this will
> contain, we expect to be able to complete before the LLVM11 branch. We
> propose this not block merging to the monorepo and we switch development
> from github to phab as soon as the merge happens. In terms of refactoring
> the code to share with Clang, our proposal is to implement the Flang side
> work then refactor later on once we understand the code better. We would be
> very happy to reconsider this and put up another RFC if we want to refactor
> as we go along instead. This approach is much easier once F18 is accepted
> into the monorepo so we can make the changes together.
>
> > > 7. No integration into the LLVM build system/Cmake
> > I don't really consider this addressed without a plan of action
(before
> merge) that contains a list of standard llvm support libraries that would
> be used in addition to things being removed (pretty much all use of C++ c*
> headers and more?) as well as staffing and approximate ETAs.
>
> Eric Schweitz has implemented this as part of his patch that adds FIR [1].
> Our plan is to extract this as a separate patch in phabricator for review
> so it can land more or less right away after F18 is merged into the
> monorepo. To be clear, our intention is that flang would *not* build by
> default after the patch.
>
> >> 8. No use of LLVM utilities or standard data structures
> >> 9. Simple deviations from the LLVM coding style
> >>     a. Separating public headers into include/flang
> >>     b. Syntactical things like braces on single line statements,
> comments on end of namespaces, etc.
> >>     c. .cc file extensions rather than .cpp
> >> 10. Bigger deviations from the LLVM coding style that are harder
to fix
> >>     a. Early exits and not using else after return, etc.
> >> 11. Flang not supporting all the same C++ compilers as the rest of
the
> project (even taking into account C++17 requirement)
> > As well as this. A plan of action isn't just a "we'll get
to this" but
> an actual plan :)
>
> We will discuss these points on the flang-dev list now and report back
> with our plans. We'll work on a list of items to fix before merge and
list
> that we propose to fix after merge on a shorter timescale. I welcome you to
> respond on those threads if there are some specific recommendations or key
> issues you care about. In any case, we'll reply back with our proposal.
>
> @Renato - I just wanted to answer a few points from your reply to make
> sure there is no misunderstanding here about F18.
>
> > Now, I am surprised it's not using LLVM API yet (a clear and
> > consistent complaint of the previous one), as well as having its own
> > driver, test infra, etc.
>
> The previous flang (to be absolutely clear - I mean this one:
> https://github.com/flang-compiler/flang) did not use the LLVM API as you
> say, instead writing a .ll file and calling clang on that file. F18 will
> certainly use LLVM APIs to generate IR directly but that code has not made
> it to the master F18 branch today. It is not true to say that F18 has
> inherited this flaw from its predecessor.
>
> > The lack of C++ compatibility may look small now, but once we want to
> > move to a newer standard, Flang will be the bottleneck. In a monorepo,
> > these things are even more important.
> > F18 was supposed to be a whole new front-end, from scratch, but it
> > looks to have carried along many of the previous projects' flaws.
>
> To clarify, F18 code is compatible to C++17 so it is currently ahead of
> the standard used by the rest of the LLVM project. That is an issue by
> itself that needs managing, but a different problem from the one you
> describe here. The problem with F18 being ahead of the rest of the project
> was discussed at length during the original upstreaming discussion
> http://lists.llvm.org/pipermail/llvm-dev/2019-February/130497.html and we
> agreed to revisit the problem when F18 is part of the project and going
> more mainstream and see where the rest of the project was in terms of its
> minimum standard. At that point we can chose what to do.
>
> Thanks again for caring about this and your help.
>
> Ta
> Rich
>
> [1] https://github.com/flang-compiler/f18/pull/920
> [2]
>
https://github.com/flang-compiler/f18/commit/a58c6067a1ab5cd02dfb5b6fb9919a20b960d984
> [3] https://reviews.llvm.org/D63607,
> https://github.com/flang-compiler/f18/pull/759,
> https://github.com/flang-compiler/f18/pull/762 and
> https://github.com/flang-compiler/f18/pull/763
>
>
> From: Eric Christopher <echristo at gmail.com>
> Sent: 13 January, 2020 06:51
> To: Richard Barton <Richard.Barton at arm.com>
> Cc: Renato Golin <rengolin at gmail.com>; Hans Wennborg <hans at
chromium.org>;
> llvm-dev at lists.llvm.org; Mike Edwards <medwards at llvm.org>
> Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!
>
>
>
> On Thu, Jan 9, 2020 at 4:28 AM Richard Barton via llvm-dev <mailto:
> llvm-dev at lists.llvm.org> wrote:
> Hi all
>
> Thanks for all the replies and engagement on this issue.
>
> First point, given the state of discussions today I would like to propose
> that we don't start the merge at 10:00 GMT on Monday 13th as proposed
and
> we delay by at least 24 hours until after the scheduled F18 technical call
> on Monday afternoon.
>
> In order to help compile a plan of action, I've tried to compile a list
of
> the concerns that folks have raised so far.
>
> I think all these have been addressed (please correct me if you think
> otherwise)
> 1. Audit trail/visibility of code review [Addressed by Peter - code has
> been reviewed [a] to F18 coding guidelines [b].
> 2. Long-term viability of Flang community and overlap with existing LLVM
> community [Hopefully Hal and Johannes replies and Greg's and Pat's
and my
> reply demonstrate long-term commitment to Flang after upstreaming]
> 3. Compatibility of license [Addressed by Steve, a recent update has made
> the licenses compatible [c]]
> 4. No use of LLVM APIs and so no connection to the project [Addressed by
> Hal and me - it is the natural next step in development as Flang starts to
> generate MLIR. Nvidia are working on this now.]
>
> The choice of MLIR is interesting - and given that there was no discussion
> of it I was wondering if you have a document laying out pros/cons and what
> ultimately made the decision here.
>
> I think these are acknowledged right now and we are actively working on
> fixes:
> 5. No use of lit in the regression tests [Arm is working on this]
> 6. Need to refactor parts of clang driver that can be shared with flang
> into a separate library [Arm is working on this, but plans to implement a
> simple driver first before refactoring to better understand the
> opportunities to do so. See Peter's RFC [d] ]
> 7. No use of LLVM utilities or standard data structures (Vector, etc.) [I
> think Pat and Eric have patches ready to go for this?]
>
> I don't really consider this addressed without a plan of action (before
> merge) that contains a list of standard llvm support libraries that would
> be used in addition to things being removed (pretty much all use of C++ c*
> headers and more?) as well as staffing and approximate ETAs.
>
>
> I think these are only acknowledged, with the intention to remediate post
> merge, but no concrete plan or owner at this point:
> 8. Simple deviations from the LLVM coding style
>     a. Separating public headers into include/flang
>     b. Syntactical things like braces on single line statements, comments
> on end of namespaces, etc.
>     c. .cc file extensions rather than .cpp
> 9. Bigger deviations from the LLVM coding style that are harder to fix
>     a. Early exits and not using else after return, etc.
> 10. Flang not supporting all the same C++ compilers as the rest of the
> project (even taking into account C++17 requirement)
> As things stand we are not proposing to remediate these (issues 5 and
> greater) in the code before it is merged to LLVM next week. These issues
> will be worked on after commit.
>
> As well as this. A plan of action isn't just a "we'll get to
this" but an
> actual plan :)
>
> Does anyone feel passionately that any of the above points need to be
> addressed in the code before we can merge?
> Does anyone feel there is anything missing from that list?
>
> Given Mehdi and Hans' query about why this needs to go into LLVM10.
Unless
> anyone has any good reason, perhaps we could postpone until after the
> branch, perhaps by one week to January 20th? That would give us some more
> time to bottom out what changes need to be made before we can accept the
> code and give us time to make any changes that block inclusion (assuming
> they are suitably sized to do in this timescale). As Renato suggests, that
> would also give us the LLVM11 branching point as a natural deadline to have
> addressed the larger concerns in our list.
>
> What do people think to that proposal?
>
> I think waiting until after the llvm 10 branch would probably be best at
> this point. That will give a lot of time for cleanup and to make f18 a much
> more reasonable "preview" including code generation in a
forthcoming
> release in about 6 mos.
>
> -eric
>
> Regards
> Rich
>
> [a] https://github.com/flang-compiler/f18/pulls
> [b]
>
https://github.com/flang-compiler/f18/blob/master/documentation/C%2B%2Bstyle.md
> [c]
>
https://github.com/flang-compiler/f18/commit/e06be2faa64a52471b3cfb2829dc05888236aa68
> [d]  http://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html
>
> > -----Original Message-----
> > From: llvm-dev <mailto:llvm-dev-bounces at lists.llvm.org> On
Behalf Of
> Renato
> > Golin via llvm-dev
> > Sent: 9 January, 2020 10:02
> > To: Hans Wennborg <mailto:hans at chromium.org>
> > Cc: llvm-dev <mailto:llvm-dev at lists.llvm.org>; Mike Edwards
<mailto:
> medwards at llvm.org>
> > Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!
> >
> > On Thu, 9 Jan 2020 at 08:13, Hans Wennborg via llvm-dev
> > <mailto:llvm-dev at lists.llvm.org> wrote:
> > > > Why is it targeting pre-llvm10 branching? Are we expecting
to ship
> > anything in LLVM 10? If so I would have expected it to land much much
> > earlier to be able to stabilize during the development phase long
before
> the
> > branching point.
> > >
> > > I was wondering about this too. I'm not necessarily opposed,
but
> > > wouldn't it make sense to target landing soon *after* the
branch
> > > instead, to give it time to integrate, stabilize, and then ship
in the
> > > next release?
> >
> > Usually, yes.
> >
> > I'm guessing this is due to downstream implementations wanting to
pick
> > up the current release as base target (which can be done regardless,
> > but...).
> >
> > In theory we can hide flang by having to explicitly name it on the
> > build command, so merging now or later won't make massive
practical
> > change.
> >
> > I personally don't like rushing things like that, but I'm not
strongly
> > opposed to either, if everyone else is on board with the rush.
> >
> > cheers,
> > --renato
> > _______________________________________________
> > LLVM Developers mailing list
> > mailto:llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> mailto: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/20200115/4f69e767/attachment.html>

Hi Eric
> FWIW my concerns are more than just "uses llvm to generate code"
and as I've outlined a few times there are a lot of specific things that you
should be using LLVM for and C like C++ is another. I did a quick look over the
code and summarized a few of the examples in an earlier post. Please try to
avoid conflating "using llvm" with "using llvm to emit IR of some
sort".
> I think that affects a lot of the "we can't give a timeline"
in your email. Can you comment further on the other code issues I've raised?
Totally understood.

We (F18 community) need to do some investigation and discuss these issues. I
would like to agree a list of such changes that would need to happen in F18 to
make it suitable to merge to the monorepo. We’ll have a better idea of
timescales after this exercise.

Expect to see some discussions starting in the coming days. It would be great if
you could join in these if you have any specific priorities to be addressed. I
will also summarise the proposal back to the list when it is agreed so we can
get input on that final list. Meanwhile, we’ll probably start implementing the
non controversial changes on the F18 side as soon as we can.

Obviously we’ll not merge on Monday 20th as previously proposed. I’ll send out a
new mail with subject line saying this.

Ta
Rich

From: Eric Christopher <echristo at gmail.com>
Sent: 15 January, 2020 21:34
To: Richard Barton <Richard.Barton at arm.com>
Cc: llvm-dev at lists.llvm.org; Renato Golin <rengolin at gmail.com>; Hans
Wennborg <hans at chromium.org>; Mike Edwards <medwards at llvm.org>
Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!

Hi Richard,

On Wed, Jan 15, 2020 at 1:11 AM Richard Barton <Richard.Barton at
arm.com<mailto:Richard.Barton at arm.com>> wrote:
Hi Eric, Renato

Thanks again for the engagement and challenge on this, it is really useful
feedback to know what we need to do to get F18 into the project in a way that
everyone is happy with.

I have tried to give timelines on the points addressed below where I can today.
Clearly we need to do some work on points 8-11, but are the above plans/answers
to points 1-7 sufficient at this stage and would not block a merge of F18 to the
monorepo? If not, what else would you need to see or what would need to be
different for you to be happy with F18 merging into the monorepo?
>> 4. No use of LLVM APIs and so no connection to the project [Addressed
by Hal and me - it is the natural next step in development as Flang starts to
generate MLIR. Nvidia are working on this now.]
> The choice of MLIR is interesting - and given that there was no discussion
of it I was wondering if you have a document laying out pros/cons and what
ultimately made the decision here.
Hopefully Steve’s reply has covered your concerns about the design choice. Also
to point out that there has been a lot of development done on this approach
already, which Eric has shared [1] (still a work in progress), and this has
reinforced our feeling that a) having a high-level IR above LLVM IR is a good
idea and b) using MLIR for that is a good idea.

Yes. That was a good post and while I haven't gotten a chance to read
everything yet I appreciate the post.

FWIW my concerns are more than just "uses llvm to generate code" and
as I've outlined a few times there are a lot of specific things that you
should be using LLVM for and C like C++ is another. I did a quick look over the
code and summarized a few of the examples in an earlier post. Please try to
avoid conflating "using llvm" with "using llvm to emit IR of some
sort".

I think that affects a lot of the "we can't give a timeline" in
your email. Can you comment further on the other code issues I've raised?

-eric

The next step for this work is to break it down into a logical stream of patches
and go through review to get them committed. Our preference would be to do this
in phabricator in the llvm project after merging. We think the benefit of doing
this review in the llvm project is that more reviewers can participate,
including those from the MLIR project. We also hope that by developing FIR
in-tree we can more easily manage its MLIR dependency.

I don’t think we can be sure of a timeline for completion of this work. In
reality adding this part of the frontend will likely be an ongoing effort over
the next months, perhaps years and it will likely be a work in progress by the
time of the LLVM11 branch. We can at least commit to starting this effort soon,
lead by Nvidia. If we don't merge to monorepo shortly then the work will
start on the F18 github project. Arm engineers will be reviewing, but we think
the benefit of doing this development in the monorepo is that other interested
parties can join in from the MLIR and LLVM projects. We'd ask that we merge
to the monorepo before the review starts.
> 5. No use of lit in the regression tests [Arm is working on this]
This effort has started already in the F18 project [2]. Arm have some work
downstream to make the next steps – porting the old tests over to use lit.
Meanwhile, we would expect make check-all to run all the new lit tests as well
as the old non-lit tests until the latter are all ported over to lit. We expect
to have this completed before the LLVM11 branch. We propose that this not gate
merge to the monorepo and we switch development from github to phab as soon as
the merge happens.
> 6. Need to refactor parts of clang driver that can be shared with flang
into a separate library [Arm is working on this, but plans to implement a simple
driver first before refactoring to better understand the opportunities to do so.
See Peter's RFC [d] ]
Similar to above, this effort has started. Arm’s next step after contributing
the flang entry point is to agree on a spec for the replacement frontend driver.
Given our expectation as to what this will contain, we expect to be able to
complete before the LLVM11 branch. We propose this not block merging to the
monorepo and we switch development from github to phab as soon as the merge
happens. In terms of refactoring the code to share with Clang, our proposal is
to implement the Flang side work then refactor later on once we understand the
code better. We would be very happy to reconsider this and put up another RFC if
we want to refactor as we go along instead. This approach is much easier once
F18 is accepted into the monorepo so we can make the changes together.
> > 7. No integration into the LLVM build system/Cmake
> I don't really consider this addressed without a plan of action (before
merge) that contains a list of standard llvm support libraries that would be
used in addition to things being removed (pretty much all use of C++ c* headers
and more?) as well as staffing and approximate ETAs.
Eric Schweitz has implemented this as part of his patch that adds FIR [1]. Our
plan is to extract this as a separate patch in phabricator for review so it can
land more or less right away after F18 is merged into the monorepo. To be clear,
our intention is that flang would *not* build by default after the patch.
>> 8. No use of LLVM utilities or standard data structures
>> 9. Simple deviations from the LLVM coding style
>>     a. Separating public headers into include/flang
>>     b. Syntactical things like braces on single line statements,
comments on end of namespaces, etc.
>>     c. .cc file extensions rather than .cpp
>> 10. Bigger deviations from the LLVM coding style that are harder to fix
>>     a. Early exits and not using else after return, etc.
>> 11. Flang not supporting all the same C++ compilers as the rest of the
project (even taking into account C++17 requirement)
> As well as this. A plan of action isn't just a "we'll get to
this" but an actual plan :)
We will discuss these points on the flang-dev list now and report back with our
plans. We'll work on a list of items to fix before merge and list that we
propose to fix after merge on a shorter timescale. I welcome you to respond on
those threads if there are some specific recommendations or key issues you care
about. In any case, we'll reply back with our proposal.

@Renato - I just wanted to answer a few points from your reply to make sure
there is no misunderstanding here about F18.
> Now, I am surprised it's not using LLVM API yet (a clear and
> consistent complaint of the previous one), as well as having its own
> driver, test infra, etc.
The previous flang (to be absolutely clear - I mean this one:
https://github.com/flang-compiler/flang) did not use the LLVM API as you say,
instead writing a .ll file and calling clang on that file. F18 will certainly
use LLVM APIs to generate IR directly but that code has not made it to the
master F18 branch today. It is not true to say that F18 has inherited this flaw
from its predecessor.
> The lack of C++ compatibility may look small now, but once we want to
> move to a newer standard, Flang will be the bottleneck. In a monorepo,
> these things are even more important.
> F18 was supposed to be a whole new front-end, from scratch, but it
> looks to have carried along many of the previous projects' flaws.
To clarify, F18 code is compatible to C++17 so it is currently ahead of the
standard used by the rest of the LLVM project. That is an issue by itself that
needs managing, but a different problem from the one you describe here. The
problem with F18 being ahead of the rest of the project was discussed at length
during the original upstreaming discussion
http://lists.llvm.org/pipermail/llvm-dev/2019-February/130497.html and we agreed
to revisit the problem when F18 is part of the project and going more mainstream
and see where the rest of the project was in terms of its minimum standard. At
that point we can chose what to do.

Thanks again for caring about this and your help.

Ta
Rich

[1] https://github.com/flang-compiler/f18/pull/920
[2]
https://github.com/flang-compiler/f18/commit/a58c6067a1ab5cd02dfb5b6fb9919a20b960d984
[3] https://reviews.llvm.org/D63607,
https://github.com/flang-compiler/f18/pull/759,
https://github.com/flang-compiler/f18/pull/762 and
https://github.com/flang-compiler/f18/pull/763


From: Eric Christopher <echristo at gmail.com<mailto:echristo at
gmail.com>>
Sent: 13 January, 2020 06:51
To: Richard Barton <Richard.Barton at arm.com<mailto:Richard.Barton at
arm.com>>
Cc: Renato Golin <rengolin at gmail.com<mailto:rengolin at
gmail.com>>; Hans Wennborg <hans at chromium.org<mailto:hans at
chromium.org>>; llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>; Mike Edwards <medwards at llvm.org<mailto:medwards at
llvm.org>>
Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!



On Thu, Jan 9, 2020 at 4:28 AM Richard Barton via llvm-dev <mailto:llvm-dev
at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
Hi all

Thanks for all the replies and engagement on this issue.

First point, given the state of discussions today I would like to propose that
we don't start the merge at 10:00 GMT on Monday 13th as proposed and we
delay by at least 24 hours until after the scheduled F18 technical call on
Monday afternoon.

In order to help compile a plan of action, I've tried to compile a list of
the concerns that folks have raised so far.

I think all these have been addressed (please correct me if you think otherwise)
1. Audit trail/visibility of code review [Addressed by Peter - code has been
reviewed [a] to F18 coding guidelines [b].
2. Long-term viability of Flang community and overlap with existing LLVM
community [Hopefully Hal and Johannes replies and Greg's and Pat's and
my reply demonstrate long-term commitment to Flang after upstreaming]
3. Compatibility of license [Addressed by Steve, a recent update has made the
licenses compatible [c]]
4. No use of LLVM APIs and so no connection to the project [Addressed by Hal and
me - it is the natural next step in development as Flang starts to generate
MLIR. Nvidia are working on this now.]

The choice of MLIR is interesting - and given that there was no discussion of it
I was wondering if you have a document laying out pros/cons and what ultimately
made the decision here.

I think these are acknowledged right now and we are actively working on fixes:
5. No use of lit in the regression tests [Arm is working on this]
6. Need to refactor parts of clang driver that can be shared with flang into a
separate library [Arm is working on this, but plans to implement a simple driver
first before refactoring to better understand the opportunities to do so. See
Peter's RFC [d] ]
7. No use of LLVM utilities or standard data structures (Vector, etc.) [I think
Pat and Eric have patches ready to go for this?]

I don't really consider this addressed without a plan of action (before
merge) that contains a list of standard llvm support libraries that would be
used in addition to things being removed (pretty much all use of C++ c* headers
and more?) as well as staffing and approximate ETAs.


I think these are only acknowledged, with the intention to remediate post merge,
but no concrete plan or owner at this point:
8. Simple deviations from the LLVM coding style
    a. Separating public headers into include/flang
    b. Syntactical things like braces on single line statements, comments on end
of namespaces, etc.
    c. .cc file extensions rather than .cpp
9. Bigger deviations from the LLVM coding style that are harder to fix
    a. Early exits and not using else after return, etc.
10. Flang not supporting all the same C++ compilers as the rest of the project
(even taking into account C++17 requirement)
As things stand we are not proposing to remediate these (issues 5 and greater)
in the code before it is merged to LLVM next week. These issues will be worked
on after commit.

As well as this. A plan of action isn't just a "we'll get to
this" but an actual plan :)

Does anyone feel passionately that any of the above points need to be addressed
in the code before we can merge?
Does anyone feel there is anything missing from that list?

Given Mehdi and Hans' query about why this needs to go into LLVM10. Unless
anyone has any good reason, perhaps we could postpone until after the branch,
perhaps by one week to January 20th? That would give us some more time to bottom
out what changes need to be made before we can accept the code and give us time
to make any changes that block inclusion (assuming they are suitably sized to do
in this timescale). As Renato suggests, that would also give us the LLVM11
branching point as a natural deadline to have addressed the larger concerns in
our list.

What do people think to that proposal?

I think waiting until after the llvm 10 branch would probably be best at this
point. That will give a lot of time for cleanup and to make f18 a much more
reasonable "preview" including code generation in a forthcoming
release in about 6 mos.

-eric

Regards
Rich

[a] https://github.com/flang-compiler/f18/pulls
[b]
https://github.com/flang-compiler/f18/blob/master/documentation/C%2B%2Bstyle.md
[c]
https://github.com/flang-compiler/f18/commit/e06be2faa64a52471b3cfb2829dc05888236aa68
[d]  http://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html
> -----Original Message-----
> From: llvm-dev <mailto:llvm-dev-bounces at
lists.llvm.org<mailto:llvm-dev-bounces at lists.llvm.org>> On Behalf Of
Renato
> Golin via llvm-dev
> Sent: 9 January, 2020 10:02
> To: Hans Wennborg <mailto:hans at chromium.org<mailto:hans at
chromium.org>>
> Cc: llvm-dev <mailto:llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>>; Mike Edwards <mailto:medwards at
llvm.org<mailto:medwards at llvm.org>>
> Subject: Re: [llvm-dev] Flang landing in the monorepo - next Monday!
>
> On Thu, 9 Jan 2020 at 08:13, Hans Wennborg via llvm-dev
> <mailto:llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>> wrote:
> > > Why is it targeting pre-llvm10 branching? Are we expecting to
ship
> anything in LLVM 10? If so I would have expected it to land much much
> earlier to be able to stabilize during the development phase long before
the
> branching point.
> >
> > I was wondering about this too. I'm not necessarily opposed, but
> > wouldn't it make sense to target landing soon *after* the branch
> > instead, to give it time to integrate, stabilize, and then ship in the
> > next release?
>
> Usually, yes.
>
> I'm guessing this is due to downstream implementations wanting to pick
> up the current release as base target (which can be done regardless,
> but...).
>
> In theory we can hide flang by having to explicitly name it on the
> build command, so merging now or later won't make massive practical
> change.
>
> I personally don't like rushing things like that, but I'm not
strongly
> opposed to either, if everyone else is on board with the rush.
>
> cheers,
> --renato
> _______________________________________________
> LLVM Developers mailing list
> mailto:llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________
LLVM Developers mailing list
mailto:llvm-dev at lists.llvm.org<mailto: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/20200117/d27a442a/attachment.html>