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

Chandler Carruth chandlerc at gmail.com
Thu Nov 1 02:56:23 PDT 2012


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.



More information about the llvm-dev mailing list