llvm dev - Jul 2016 - Target Acceptance Policy

> On Jul 26, 2016, at 2:58 AM, Renato Golin <renato.golin at
linaro.org> wrote:
> 
> On 26 July 2016 at 04:46, Mehdi Amini <mehdi.amini at apple.com>
wrote:
>> I don’t make sense of this, so I guess we are not talking about the
same
>> thing.
> 
> I think I know what it is... :)
> 
> 
>> Moving from experimental to non-experimental is a one-line patch
>> while all the pieces are glued together in-tree, and I don’t see any
>> practical way to have some modular review of a backend *after* this
>> happened.
> 
> You're assuming there will be no code review on the back-end and
we'll
> accept spaghetti code because the "policy" doesn't say
anything about
> it. I understand why you see it that way, as it's not explicit.
No, the problem is that your writing is making an exception to the developer
policy, and I don’t think it is a good thing.
> 
> But as with all policies, we'd better encode the bare minimum than the
> most complete picture, as there will always be edge cases we can't
> predict.
There is no edge cases in question here.
> What *normally* happens is that, during the first review (bulk
> import), people point out several problems with the code. Style
> issues, bad tests, bad patterns, misconceptions, etc. All of those are
> generally fixed pre-import, if they're simple, or marked as FIXME if
> more elaborate.
What kind of conformance to "developers policy as well as the coding
standards” is too elaborate to be fixed pre-import?
I don’t think any, so I don’t see why this should be deferred.

> There is no ratio for that, it can be all easy, or
> many hard, and it will depend on the case. If we make all rules fixed
> at maximum, we will lose the ability to import code and evolve with
> time.
> 
> One of the key points, and the first one indeed, is that the community
> has to be pro-active and nice. If we request a bunch of changes, they
> don't change and request to be made official
I have a totally different opinion: they submit a patch, we request changes, and
if they don’t the code never gets in tree in the first place.
> , then they have failed
> the first and most important rule.
> 
> OTOH, if we failed to spot the problems in the first bulk, and later
> accept a bad target as official, than it would be our own fault.
Asking as much separate patches as possible is part of making sure it does not
happen.

— 
Mehdi

> 
> We can add a note describing the FIXME approach if people think it
> will make things more clear, but essentially, the intent is in there
> already. I'm ok with being more explicit, though, just not more
> strict.
> 
> cheers,
> --renato

On 26 July 2016 at 17:50, Mehdi Amini <mehdi.amini at apple.com>
wrote:
> No, the problem is that your writing is making an exception to the
developer policy, and I don’t think it is a good thing.
I think requiring such a high bar from start is not a good community
sportsmanship.

I want to help other people to understand and follow our guidelines,
and the best place to do this, is to have an experimental stage, where
the back-end is in tree but not officially built. This gives them time
to address all comments with our help. We want targets as much as they
want to be in LLVM. It's a cooperation and acting inflexibly won't
help anyone.

Also, if we put all the blockages in the first stop, what's the point
of being experimental in the first place?

> There is no edge cases in question here.
Chandler has found a number of edge cases on my original (more strict)
writing. This is a revised text, and by all means, will have cases
that we didn't foresee.

> I have a totally different opinion: they submit a patch, we request
changes, and if they don’t the code never gets in tree in the first place.
I hear, loud and clear. I also understand the reasons. I just don't
share your opinion that this is valid in *every* case.

We usually reserved strong policies for critical problems (legal,
license, patents, copyright), and less strict ones for more common
sense kind of things. May not be perfect, but it's a balance.

cheers,
--renato

> On Jul 26, 2016, at 10:28 AM, Renato Golin <renato.golin at
linaro.org> wrote:
> 
> On 26 July 2016 at 17:50, Mehdi Amini <mehdi.amini at apple.com>
wrote:
>> No, the problem is that your writing is making an exception to the
developer policy, and I don’t think it is a good thing.
> 
> I think requiring such a high bar from start is not a good community
> sportsmanship.
Yet this is exactly what we do for any new contributor that wants to add a new
pass in the compiler for instance, even if it is not enabled in the default
pipeline (which is equivalent to the experimental stage somehow).

> 
> I want to help other people to understand and follow our guidelines,
> and the best place to do this, is to have an experimental stage, where
> the back-end is in tree but not officially built.
I disagree, the best place for that is with a patch reviewed.

It is very hard to evaluate how much coverage components gets with lit-based
test without doing it while integrating patches.
I just don’t see any viable alternative, re-evaluating the full target and its
tests every other month is just *not* practical.

I you don’t have a good way to integrate the patches in good conditions (i.e.
with conferment coding style and good tests), you’ll never get there later (or
at very high cost).

> This gives them time
> to address all comments with our help. We want targets as much as they
> want to be in LLVM. It's a cooperation and acting inflexibly won't
> help anyone.
I believe that it helps everyone on the opposite.
> 
> Also, if we put all the blockages in the first stop, what's the point
> of being experimental in the first place?
All the rest of what you described in (show that the maintainer is active,
etc.), and also, waiting for the implementation to be “complete” (as in “all
patches integrated”).

— 
Mehdi

> 
> 
>> There is no edge cases in question here.
> 
> Chandler has found a number of edge cases on my original (more strict)
> writing. This is a revised text, and by all means, will have cases
> that we didn't foresee.
> 
> 
>> I have a totally different opinion: they submit a patch, we request
changes, and if they don’t the code never gets in tree in the first place.
> 
> I hear, loud and clear. I also understand the reasons. I just don't
> share your opinion that this is valid in *every* case.
> 
> We usually reserved strong policies for critical problems (legal,
> license, patents, copyright), and less strict ones for more common
> sense kind of things. May not be perfect, but it's a balance.
> 
> cheers,
> --renato