[LLVMdev] Heads up, I've backed out significant amounts of the multiple address space conversion changes

[Villmow, Micah]

First off I want to thank everyone for helping out with this. While I don't like that the code was backed out, I would do the same thing in your shoes given the same situation.

So, what should the next step be? I'm still catching up on what exactly is still there and what was removed, but that shouldn't stop the discussion from continuing.

Micah 
> -----Original Message-----
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
> On Behalf Of Chandler Carruth
> Sent: Thursday, November 01, 2012 2:56 AM
> To: LLVM Developers Mailing List; Micah Villmow; Duncan Sands
> Subject: [LLVMdev] Heads up, I've backed out significant amounts of the
> multiple address space conversion changes
> 
> TL;DR: See subject. That is all.
> ----
> 
> First off, I want to say I'm sorry for doing this. =/
> 
> There were several issues that led to this:
> 
> 1) There were several micro-design problems with the APIs as
> implemented in the core libraries of LLVM. These then fanned out to
> all the uses of these libraries making fixing these issues challenging
> as well. The fixes were also not happening in a timely fashion, and
> instead a lot of code was switched over to the not-yet-right
> interfaces.
> 
> 2) The updates to the broader libraries using the pointer types and
> address spaces were not really well factored to use all of the helper
> and convenience routines available, and add ones where missing. Some
> helpers were added, but not always enough, and not always in time for
> some of the usages to reflect these helpers.
> 
> 3) The updates to the broader libraires, especially the transforms and
> analysis libraries were never accompanied with test cases for those
> transforms or analyses to even sanity check the changes. A few of
> these had bugs spotted by inspection during post-commit review, but we
> don't even really know how many more might be lurking.
> 
> 4) The commits were poorly factored into isolated, incremental chunks.
> Notably, reworking of core APIs was mixed with updating their
> consumers, and with general cleanups. This makes isolated reverts
> impossible, and general reverts increasingly hard as time passes.
> 
> 5) There is at least one serious bug with the methodology being
> employed in some of the usage patterns, leading to PR14233.
> 
> 6) Micah is out of the office for a while and not likely to have the
> time necessary to address these issues quickly.
> 
> 7) We're going to be branching for 3.2 in not-too-long.
> 
> 
> When you combine all these factors, the best option I saw was to back
> out the widespread changes outside of the core libraries, and some of
> the more problematic changes to the core libraries, leaving as much of
> the work on DataLayout etc in tree as possible. This fixes PR14233, my
> immediate need as this is a serious regression, and it paves the way
> for a relatively safe state to release 3.2 in even if Micah doesn't
> get significant time to work on this prior to the release branching.
> 
> I didn't make this decision in isolation though. I talked extensively
> about these issues with Nick Lewycky, Richard Smith, Benjamin Kramer,
> and Duncan Sands. Duncan and I made the final call to pull these
> patches out once I heard that Micah was going to be out and unable te
> quickly address an PRs, etc.
> 
> Since I made the call to pull it out, I did the work to disentangle
> the commits, and have backed it out in a series of commits tonight.
> I'll plan on doing some of the cleanups and API fixes in the core
> libraries as well to ensure those are in place prior to 3.2.
> 
> 
> Some things I think are going to be pretty important when you try to
> return to this Micah in order to have a bit more stability and success
> getting this into the tree and staying there:
> 
> - I think we should spend some time iterating on the core APIs used to
> query, test, set and manipulate the address space and pointer types
> with address spaces. Until these interfaces look really good, you
> shouldn't convert many users.
> 
> - Any conversion changes that just switch usage patterns should be
> separate commits from any changes to the APIs being used. Any generic
> cleanups that remove even the need for a conversion change should also
> be in their own independent commit.
> 
> - You should keep a running patch that converts a few core passes and
> bits of LLVM to use the new address space APIs, and use that to figure
> out the issues with the core APIs.
> 
> - Each of the changes to a core API like DataLayout should (where
> possible) be tested directly via a unittest.
> 
> - Each of the conversions for a pass or target should be accompanied
> by test cases with multiple address spaces so that other developers
> don't blindly break the support constructs implemented.
> 
> 
> Anyways, happy to talk more about how to go about making these
> sweeping changes when you're back Micah.
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev