llvm dev - Dec 2013 - [LLVMdev] running clang format on the Mips target

Hi David,

What kind of  "a lot of out-of-tree changes"?

You should push changes incrementally as you do work. Holding onto changes means
that many things,
not just reformatting, can make them need to be redone.  We frequently clean up
and rewrite
code to make it cleaner and easier to maintain.

We are moving to a more strict internal review and pushing of changes and post
commit reviewing.

It takes time to review and respond to comments on formatting issues; time that
would be better spent doing new work.

So we would like to have robots, i.e. clang format, do this checking and such.

I would recommend that you start to submit your patches for review.


Reed

________________________________________
From: Dr D. Chisnall [dc552 at hermes.cam.ac.uk] on behalf of David Chisnall
[David.Chisnall at cl.cam.ac.uk]
Sent: Saturday, December 21, 2013 6:43 AM
To: Reed Kotler
Cc: LLVMdev at cs.uiuc.edu
Subject: Re: [LLVMdev] running clang format on the Mips target

As someone with an  lot of out-of-tree changes to the MIPS back end, I would
consider this to be somewhat impolite.  I hope to upstream the changes that are
relevant to users of other MIPS-derived processors in the new year, and having
large numbers of formatting changes would make rebasing a lot more painful.

David

On 20 Dec 2013, at 20:52, reed kotler <rkotler at mips.com> wrote:
> We are considering running clang format on the whole Mips target.
>
> Is there any rule against this?
>
> Is there any good argument against doing this even if there is no rule
against it?
>
> TIA.
>
> Reed
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

Hi Reed,

On 21 Dec 2013, at 19:20, Reed Kotler <Reed.Kotler at imgtec.com> wrote:
> Hi David,
> 
> What kind of  "a lot of out-of-tree changes"?
Support for a new (experimental) processor architecture, which is not likely to
be  upstreamed because it's not of interest to anyone who isn't using
our architecture.  While these changes are public, there is no point in
upstreaming them (they also include some hacks in the middle parts of LLVM to
support fat pointers, which might be cleaned up and upstreamed at some point if
anyone apart from us is interested in fat pointers), because it's a rapidly
changing experimental ISA.  Our processor runs FreeBSD, and so I've also
been fixing things that are required to let us move the FreeBSD/MIPS build from
gcc 4.2.1 to clang.
> You should push changes incrementally as you do work. Holding onto changes
means that many things,
> not just reformatting, can make them need to be redone.  We frequently
clean up and rewrite
> code to make it cleaner and easier to maintain.
> 
> We are moving to a more strict internal review and pushing of changes and
post commit reviewing.
I have had to fix a large number of bugs in the MIPS back end to allow us to
build parts of the FreeBSD kernel and userland with LLVM.  It would be nice if
you had worked on getting the basic functionality working before chasing
microMIPS and the various ASE things.

For us, the first priority is getting things to work with our branch.  Once
we've got things into a state where it's tested, we'll start
upstreaming things.  Our code is on github and Jack has the URL, so feel free to
pull in anything you want in the meantime, or if there are specific revisions
that you'd like me to clean up and rebase I'd be happy to do so.  In
particular, we have dsub* and daddi, dli, dla, la, and .cpsetup implemented
(although not the most efficient implementation of any of them, hence these
patches not being ready for upstreaming), .set noat / at doing the right thing,
and have fixed (I think) MIPS IV support (although not yet MIPS III, which is an
oversight given that Loongson 2F is MIPS III).
> It takes time to review and respond to comments on formatting issues; time
that would be better spent doing new work.
Has anyone done a code review of the existing MIPS back end?  
> So we would like to have robots, i.e. clang format, do this checking and
such.
> 
> I would recommend that you start to submit your patches for review.
I intend to in the new year (and I've already mentioned this to Jack and
Vladimir), but only the ones that are relevant for other MIPS consumers.  Given
the large number of non-standard extensions to MIPS that abound now (and this
will only increase when we open source our softcore in the new year), I imagine
that there will be no shortage of other MIPS-based back ends being maintained
out of tree.  The poor modularity in the existing code makes it difficult to
upstream these, although I've been (very slowly) working on fixing that in
our branch.

David

Daniel Sanders is the official maintainer for the Mips LLVM port. 

If you are finding bugs, you should be filing them. We even have an open facing
bugzilla for Mips. It would be
possible to arrange you to have an account there. Or you could file them in the
official LLVM bugzilla.

If you want to help and fix them yourself or contribute other work; that's
great and is very welcome.

When you do that; you should make a patch and push them upstream.

I seriously doubt that anyone at imagination/Mips is going to spend time digging
though some external Git
repository to glean possible changes.

At this time you don't have pre-commit privilege so you need to submit them
for review, like we all needed to in the beginning including Akira and myself,
and get them approved. After some time it's possible to get promoted to post
commit review. All of us are subject to post-commit review and per the rules of
LLVM, Daniel, being the Mips maintainer, is the final word on any changes to the
Mips target and all of us have to adhere to that.

There are procedures in LLVM for all these things and nobody is exempt from
them.

Our compiler passes test-suite for Mips32, mips16 and micro mips. It can recurse
itself and work on all the
standard benchmarks, plumhall c/c++/c++ library and many other things. Several
years back, someone from outside
of Mips, who is not connected to Mips in any way, tested all the compilers with
some networking test suites and code and the Mips compiler was the only one at
that time that passed 100% out of the box.

Does it have bugs? Most likely unless physics somehow works differently for us. 
We run our own build bots on many flavors of the compiler
every night and it's a "stop all new work" whenever any of these
variants goes red.

All compilers (and software) have bugs otherwise nobody would have invented
bugzilla or any of the hundreds of other bug tracking programs.

If you have patches that would help clean up parts of the compiler; these are
also welcome but you have to go through
the procedures that we all have to go through.

If it's something major, then you would be advised to submit an RFC and get
some kind of buy in from the rest of the
Mips team before doing them.

Reed
________________________________________
From: Dr D. Chisnall [dc552 at hermes.cam.ac.uk] on behalf of David Chisnall
[David.Chisnall at cl.cam.ac.uk]
Sent: Saturday, December 21, 2013 12:06 PM
To: Reed Kotler
Cc: LLVMdev at cs.uiuc.edu
Subject: Re: [LLVMdev] running clang format on the Mips target

Hi Reed,

On 21 Dec 2013, at 19:20, Reed Kotler <Reed.Kotler at imgtec.com> wrote:
> Hi David,
>
> What kind of  "a lot of out-of-tree changes"?
Support for a new (experimental) processor architecture, which is not likely to
be  upstreamed because it's not of interest to anyone who isn't using
our architecture.  While these changes are public, there is no point in
upstreaming them (they also include some hacks in the middle parts of LLVM to
support fat pointers, which might be cleaned up and upstreamed at some point if
anyone apart from us is interested in fat pointers), because it's a rapidly
changing experimental ISA.  Our processor runs FreeBSD, and so I've also
been fixing things that are required to let us move the FreeBSD/MIPS build from
gcc 4.2.1 to clang.
> You should push changes incrementally as you do work. Holding onto changes
means that many things,
> not just reformatting, can make them need to be redone.  We frequently
clean up and rewrite
> code to make it cleaner and easier to maintain.
>
> We are moving to a more strict internal review and pushing of changes and
post commit reviewing.
I have had to fix a large number of bugs in the MIPS back end to allow us to
build parts of the FreeBSD kernel and userland with LLVM.  It would be nice if
you had worked on getting the basic functionality working before chasing
microMIPS and the various ASE things.

For us, the first priority is getting things to work with our branch.  Once
we've got things into a state where it's tested, we'll start
upstreaming things.  Our code is on github and Jack has the URL, so feel free to
pull in anything you want in the meantime, or if there are specific revisions
that you'd like me to clean up and rebase I'd be happy to do so.  In
particular, we have dsub* and daddi, dli, dla, la, and .cpsetup implemented
(although not the most efficient implementation of any of them, hence these
patches not being ready for upstreaming), .set noat / at doing the right thing,
and have fixed (I think) MIPS IV support (although not yet MIPS III, which is an
oversight given that Loongson 2F is MIPS III).
> It takes time to review and respond to comments on formatting issues; time
that would be better spent doing new work.
Has anyone done a code review of the existing MIPS back end?
> So we would like to have robots, i.e. clang format, do this checking and
such.
>
> I would recommend that you start to submit your patches for review.
I intend to in the new year (and I've already mentioned this to Jack and
Vladimir), but only the ones that are relevant for other MIPS consumers.  Given
the large number of non-standard extensions to MIPS that abound now (and this
will only increase when we open source our softcore in the new year), I imagine
that there will be no shortage of other MIPS-based back ends being maintained
out of tree.  The poor modularity in the existing code makes it difficult to
upstream these, although I've been (very slowly) working on fixing that in
our branch.

David

Hi David,

I agree with you that it would be rude to simply clang-format the MIPS backend
without coordination with any out-of-tree derivatives. To be honest, it
hadn't occurred to me that there would be any such derivatives and the
possibility wasn't raised in our internal discussion before we brought the
subject up on this list. I'm keen to coordinate with such derivatives to
minimize disruption as far as is reasonably possible. It's not going to be
possible to completely eliminate all disruption (and as Reed notes, general
development can also cause this disruption) but we can do our best.

I'm keen to get the bugfixes you mention upstreamed. Some of my colleagues
have had a look at your git repo and tell me that some of the bugfixes lack
testcases but should otherwise be ok to upstream. It also sounds like you have
working MIPS-IV support. If this is the case then I'm keen to get that
upstreamed as well since it will go some way towards properly supporting the
older ISA's (MIPS-II and MIPS-III in particular) in use by some Linux
distro's, OpenBSD, etc.
> Has anyone done a code review of the existing MIPS back end?
Yes, all patches are reviewed either before or after commit. A wider review of
the backend would be a sensible thing to do at some point in the near future.
However, I have to balance the desirability of a perfect design and
implementation with impracticality of achieving it and the business needs of our
company. I will therefore need to work this around existing schedules and
deadlines without disrupting them. At the moment, my aim is to free up
developer-time from style issues in patches. The time saved in this area is
likely to be significant since our team is split between three/four timezones
and as a result discussions about patches can take multiple days.

I'm also keen to hear specific criticisms from outside our team. Could you
elaborate on the issues you mention?
________________________________________
From: llvmdev-bounces at cs.uiuc.edu [llvmdev-bounces at cs.uiuc.edu] on behalf
of David Chisnall [David.Chisnall at cl.cam.ac.uk]
Sent: 21 December 2013 20:06
To: Reed Kotler
Cc: LLVMdev at cs.uiuc.edu
Subject: Re: [LLVMdev] running clang format on the Mips target

Hi Reed,

On 21 Dec 2013, at 19:20, Reed Kotler <Reed.Kotler at imgtec.com> wrote:
> Hi David,
>
> What kind of  "a lot of out-of-tree changes"?
Support for a new (experimental) processor architecture, which is not likely to
be  upstreamed because it's not of interest to anyone who isn't using
our architecture.  While these changes are public, there is no point in
upstreaming them (they also include some hacks in the middle parts of LLVM to
support fat pointers, which might be cleaned up and upstreamed at some point if
anyone apart from us is interested in fat pointers), because it's a rapidly
changing experimental ISA.  Our processor runs FreeBSD, and so I've also
been fixing things that are required to let us move the FreeBSD/MIPS build from
gcc 4.2.1 to clang.
> You should push changes incrementally as you do work. Holding onto changes
means that many things,
> not just reformatting, can make them need to be redone.  We frequently
clean up and rewrite
> code to make it cleaner and easier to maintain.
>
> We are moving to a more strict internal review and pushing of changes and
post commit reviewing.
I have had to fix a large number of bugs in the MIPS back end to allow us to
build parts of the FreeBSD kernel and userland with LLVM.  It would be nice if
you had worked on getting the basic functionality working before chasing
microMIPS and the various ASE things.

For us, the first priority is getting things to work with our branch.  Once
we've got things into a state where it's tested, we'll start
upstreaming things.  Our code is on github and Jack has the URL, so feel free to
pull in anything you want in the meantime, or if there are specific revisions
that you'd like me to clean up and rebase I'd be happy to do so.  In
particular, we have dsub* and daddi, dli, dla, la, and .cpsetup implemented
(although not the most efficient implementation of any of them, hence these
patches not being ready for upstreaming), .set noat / at doing the right thing,
and have fixed (I think) MIPS IV support (although not yet MIPS III, which is an
oversight given that Loongson 2F is MIPS III).
> It takes time to review and respond to comments on formatting issues; time
that would be better spent doing new work.
Has anyone done a code review of the existing MIPS back end?
> So we would like to have robots, i.e. clang format, do this checking and
such.
>
> I would recommend that you start to submit your patches for review.
I intend to in the new year (and I've already mentioned this to Jack and
Vladimir), but only the ones that are relevant for other MIPS consumers.  Given
the large number of non-standard extensions to MIPS that abound now (and this
will only increase when we open source our softcore in the new year), I imagine
that there will be no shortage of other MIPS-based back ends being maintained
out of tree.  The poor modularity in the existing code makes it difficult to
upstream these, although I've been (very slowly) working on fixing that in
our branch.

David


_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev