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

[Matt Arsenault]

I guess I'll try picking this back up. What were the problems exactly from a design perspective?

On Nov 1, 2012, at 2:56 , Chandler Carruth <chandlerc at gmail.com> wrote:

> 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