[LLVMdev] running clang format on the Mips target

[Daniel Sanders]

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