[PATCH 1/9] R600/SI: fix stupid typo

[David Blaikie]

On Feb 26, 2013 2:20 AM, "Christian König" <deathsimple at vodafone.de> wrote:
>
> Am 26.02.2013 03:33, schrieb Sean Silva:
>>
>> On Mon, Feb 25, 2013 at 3:00 PM, Tom Stellard <tom at stellard.net> wrote:
>>>
>>> Can you clarify this?  Do you mean one email with with all the patches
>>> as attachments?
>>
>>
>> That would be preferable. Note that gmail (which many of us use) doesn't
respect threading (instead it arbitrarily threads based on subject for some
reason), so having them all as replies will still flood the inbox of gmail
users.
>>
>
> That's the exactly opposite policy as many other open source development
list use, the argument pro in-lining patches is usually that the code is
easier to comment on.

Yep, different communities have different processes. I don't personally
mind a combination of both (attachment and inline) though one of the
reasons we prefer to have the attachment (even if there's also an inline
copy) is that it's authoritative. All whitespace is preserved, no email
server induced line wrapping, etc (and we're kind of pedantic about
formatting, trailing whitespace, etc, so it's important to have an
immutable, exact copy)

Alternatively, we do have the phabricator code review system setup to
assist reviewing too. (Which provides some of the benefits of both inline
and authoritative patch content)

> But personally I usually also prefer some form of one mail (or one
thread) per patchset for review.

The issue of what to do with a patchset is still a bit open/uncertain in
the llvm community. I for one don't /entirely/ mind one email per patch
because it can be easier to track which parts of the set have been
resolved/committed.

>
>> However, it is much more in line with LLVM development style to send in
patches incrementally for review rather than dumping a whole branch; that
ensures focused review.
>
>
> Those patches belong together and either implement new features or fix
bugs that were triggered/found while implementing those new features.
Reviewing them separately would just increase the chance of missing
something regarding inter patch dependency.

There should not be circular dependency, one way dependence should be
resolved by sending the dependent patches first, as they are developed.
They should stand in their own right - though may require some narrative to
explain where they fit in a longer term plan.

Though of course this is not always the case and sometimes you don't know
where a patch set will start until you get to the end. In which case you
have a whole patch set ready for review/commit at the same time. Sending
that in one batch is what it is - but when possible it's nice to avoid such
a dump.

>
>
>> (I'll also reiterate how troubling it is that none of these R600 changes
have unit tests).
>
>
> Totally agree on that, the major problem seems to be that currently the
instruction scheduler and/or register allocator produce different results
when compiling the same code twice. The results seems to be always correct,
it's just not very well comparable. That fact makes it quite difficult for
me to write profound unit tests. Any ideas on that?

Any case of nondeterminism is a serious bug. Please provide repro steps.

>
> Christian.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130226/01ea6c33/attachment.html>