#
dab4eae2 |
| 23-Nov-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Change the static object whose address is used to uniquely identify analyses to have a common type which is enforced rather than using a char object and a `void *` type when used as an identifie
[PM] Change the static object whose address is used to uniquely identify analyses to have a common type which is enforced rather than using a char object and a `void *` type when used as an identifier.
This has a number of advantages. First, it at least helps some of the confusion raised in Justin Lebar's code review of why `void *` was being used everywhere by having a stronger type that connects to documentation about this.
However, perhaps more importantly, it addresses a serious issue where the alignment of these pointer-like identifiers was unknown. This made it hard to use them in pointer-like data structures. We were already dodging this in dangerous ways to create the "all analyses" entry. In a subsequent patch I attempted to use these with TinyPtrVector and things fell apart in a very bad way.
And it isn't just a compile time or type system issue. Worse than that, the actual alignment of these pointer-like opaque identifiers wasn't guaranteed to be a useful alignment as they were just characters.
This change introduces a type to use as the "key" object whose address forms the opaque identifier. This both forces the objects to have proper alignment, and provides type checking that we get it right everywhere. It also makes the types somewhat less mysterious than `void *`.
We could go one step further and introduce a truly opaque pointer-like type to return from the `ID()` static function rather than returning `AnalysisKey *`, but that didn't seem to be a clear win so this is just the initial change to get to a reliably typed and aligned object serving is a key for all the analyses.
Thanks to Richard Smith and Justin Lebar for helping pick plausible names and avoid making this refactoring many times. =] And thanks to Sean for the super fast review!
While here, I've tried to move away from the "PassID" nomenclature entirely as it wasn't really helping and is overloaded with old pass manager constructs. Now we have IDs for analyses, and key objects whose address can be used as IDs. Where possible and clear I've shortened this to just "ID". In a few places I kept "AnalysisID" to make it clear what was being identified.
Differential Revision: https://reviews.llvm.org/D27031
llvm-svn: 287783
show more ...
|
#
dc288a89 |
| 26-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Refactor this unittest a bit to remove duplicated code. This was suggested at one point during code review and I deferred it to a follow-up commit.
llvm-svn: 282383
|
#
e35f84a2 |
| 26-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Add a unittest covering the invalidation of a Module analysis from a function pass nested inside of a CGSCC pass manager.
This is very similar to the previous unittest but makes sure the invali
[PM] Add a unittest covering the invalidation of a Module analysis from a function pass nested inside of a CGSCC pass manager.
This is very similar to the previous unittest but makes sure the invalidation logic works across all the layers here.
llvm-svn: 282378
show more ...
|
#
b52b573d |
| 26-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Add a unittest for invalidating module analyses with an SCC pass.
This reinstates r280447. Original commit log: This wasn't really well explicitly tested with a nice unittest before. It seems g
[PM] Add a unittest for invalidating module analyses with an SCC pass.
This reinstates r280447. Original commit log: This wasn't really well explicitly tested with a nice unittest before. It seems good to have reasonably broken out unittests for this kind of functionality as I'm workin go other invalidation features to make sure none of the existing ones regress.
This still has too much duplicated code, I plan to factor that out in a subsequent commit to use common helpers for repeated parts of this.
llvm-svn: 282377
show more ...
|
#
ccd44939 |
| 04-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Revert r280447: Add a unittest for invalidating module analyses with an SCC pass.
This was mistakenly committed. The world isn't ready for this test, the test code has horrible debugging code i
[PM] Revert r280447: Add a unittest for invalidating module analyses with an SCC pass.
This was mistakenly committed. The world isn't ready for this test, the test code has horrible debugging code in it that should never have landed in tree, it currently passes because of bugs elsewhere, and it needs to be rewritten to not be susceptible to passing for the wrong reasons.
I'll re-land this in a better form when the prerequisite patches land.
So sorry that I got this mixed into a series of commits that *were* ready to land. I shouldn't have. =[ What's worse is that it stuck around for so long and I discovered it while fixing the underlying bug that caused it to pass.
llvm-svn: 280620
show more ...
|
#
0f0ef132 |
| 02-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Try to fix an MSVC2013 failure due to finding a template constructor when trying to do copy construction by adding an explicit move constructor.
Will watch the bots to discover if this is suffi
[PM] Try to fix an MSVC2013 failure due to finding a template constructor when trying to do copy construction by adding an explicit move constructor.
Will watch the bots to discover if this is sufficient.
llvm-svn: 280479
show more ...
|
#
c906ff63 |
| 02-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Add a unittest for invalidating module analyses with an SCC pass.
This wasn't really well explicitly tested with a nice unittest before. It seems good to have reasonably broken out unittests fo
[PM] Add a unittest for invalidating module analyses with an SCC pass.
This wasn't really well explicitly tested with a nice unittest before. It seems good to have reasonably broken out unittests for this kind of functionality as I'm workin go other invalidation features to make sure none of the existing ones regress.
This still has too much duplicated code, I plan to factor that out in a subsequent commit to use common helpers for repeated parts of this.
llvm-svn: 280447
show more ...
|
#
4f83742a |
| 02-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] (NFC) Split the IR parsing into a fixture so that I can split out more testing into other test routines while using the same core module.
llvm-svn: 280446
|
#
d4e80a96 |
| 02-Sep-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] (NFC) Refactor the CGSCC pass manager tests to use lambda-based passes.
This simplifies the test some and makes it more focused and clear what is being tested. It will also make it much easier
[PM] (NFC) Refactor the CGSCC pass manager tests to use lambda-based passes.
This simplifies the test some and makes it more focused and clear what is being tested. It will also make it much easier to extend with further testing of different pass behaviors.
I've also replaced a pointless module pass with running the requires pass directly as that is all that it was really doing.
llvm-svn: 280444
show more ...
|
Revision tags: llvmorg-3.9.0, llvmorg-3.9.0-rc3 |
|
#
88823468 |
| 24-Aug-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Introduce basic update capabilities to the new PM's CGSCC pass manager, including both plumbing and logic to handle function pass updates.
There are three fundamentally tied changes here: 1) Pl
[PM] Introduce basic update capabilities to the new PM's CGSCC pass manager, including both plumbing and logic to handle function pass updates.
There are three fundamentally tied changes here: 1) Plumbing *some* mechanism for updating the CGSCC pass manager as the CG changes while passes are running. 2) Changing the CGSCC pass manager infrastructure to have support for the underlying graph to mutate mid-pass run. 3) Actually updating the CG after function passes run.
I can separate them if necessary, but I think its really useful to have them together as the needs of #3 drove #2, and that in turn drove #1.
The plumbing technique is to extend the "run" method signature with extra arguments. We provide the call graph that intrinsically is available as it is the basis of the pass manager's IR units, and an output parameter that records the results of updating the call graph during an SCC passes's run. Note that "...UpdateResult" isn't a *great* name here... suggestions very welcome.
I tried a pretty frustrating number of different data structures and such for the innards of the update result. Every other one failed for one reason or another. Sometimes I just couldn't keep the layers of complexity right in my head. The thing that really worked was to just directly provide access to the underlying structures used to walk the call graph so that their updates could be informed by the *particular* nature of the change to the graph.
The technique for how to make the pass management infrastructure cope with mutating graphs was also something that took a really, really large number of iterations to get to a place where I was happy. Here are some of the considerations that drove the design:
- We operate at three levels within the infrastructure: RefSCC, SCC, and Node. In each case, we are working bottom up and so we want to continue to iterate on the "lowest" node as the graph changes. Look at how we iterate over nodes in an SCC running function passes as those function passes mutate the CG. We continue to iterate on the "lowest" SCC, which is the one that continues to contain the function just processed.
- The call graph structure re-uses SCCs (and RefSCCs) during mutation events for the *highest* entry in the resulting new subgraph, not the lowest. This means that it is necessary to continually update the current SCC or RefSCC as it shifts. This is really surprising and subtle, and took a long time for me to work out. I actually tried changing the call graph to provide the opposite behavior, and it breaks *EVERYTHING*. The graph update algorithms are really deeply tied to this particualr pattern.
- When SCCs or RefSCCs are split apart and refined and we continually re-pin our processing to the bottom one in the subgraph, we need to enqueue the newly formed SCCs and RefSCCs for subsequent processing. Queuing them presents a few challenges: 1) SCCs and RefSCCs use wildly different iteration strategies at a high level. We end up needing to converge them on worklist approaches that can be extended in order to be able to handle the mutations. 2) The order of the enqueuing need to remain bottom-up post-order so that we don't get surprising order of visitation for things like the inliner. 3) We need the worklists to have set semantics so we don't duplicate things endlessly. We don't need a *persistent* set though because we always keep processing the bottom node!!!! This is super, super surprising to me and took a long time to convince myself this is correct, but I'm pretty sure it is... Once we sink down to the bottom node, we can't re-split out the same node in any way, and the postorder of the current queue is fixed and unchanging. 4) We need to make sure that the "current" SCC or RefSCC actually gets enqueued here such that we re-visit it because we continue processing a *new*, *bottom* SCC/RefSCC.
- We also need the ability to *skip* SCCs and RefSCCs that get merged into a larger component. We even need the ability to skip *nodes* from an SCC that are no longer part of that SCC.
This led to the design you see in the patch which uses SetVector-based worklists. The RefSCC worklist is always empty until an update occurs and is just used to handle those RefSCCs created by updates as the others don't even exist yet and are formed on-demand during the bottom-up walk. The SCC worklist is pre-populated from the RefSCC, and we push new SCCs onto it and blacklist existing SCCs on it to get the desired processing.
We then *directly* update these when updating the call graph as I was never able to find a satisfactory abstraction around the update strategy.
Finally, we need to compute the updates for function passes. This is mostly used as an initial customer of all the update mechanisms to drive their design to at least cover some real set of use cases. There are a bunch of interesting things that came out of doing this:
- It is really nice to do this a function at a time because that function is likely hot in the cache. This means we want even the function pass adaptor to support online updates to the call graph!
- To update the call graph after arbitrary function pass mutations is quite hard. We have to build a fairly comprehensive set of data structures and then process them. Fortunately, some of this code is related to the code for building the cal graph in the first place. Unfortunately, very little of it makes any sense to share because the nature of what we're doing is so very different. I've factored out the one part that made sense at least.
- We need to transfer these updates into the various structures for the CGSCC pass manager. Once those were more sanely worked out, this became relatively easier. But some of those needs necessitated changes to the LazyCallGraph interface to make it significantly easier to extract the changed SCCs from an update operation.
- We also need to update the CGSCC analysis manager as the shape of the graph changes. When an SCC is merged away we need to clear analyses associated with it from the analysis manager which we didn't have support for in the analysis manager infrsatructure. New SCCs are easy! But then we have the case that the original SCC has its shape changed but remains in the call graph. There we need to *invalidate* the analyses associated with it.
- We also need to invalidate analyses after we *finish* processing an SCC. But the analyses we need to invalidate here are *only those for the newly updated SCC*!!! Because we only continue processing the bottom SCC, if we split SCCs apart the original one gets invalidated once when its shape changes and is not processed farther so its analyses will be correct. It is the bottom SCC which continues being processed and needs to have the "normal" invalidation done based on the preserved analyses set.
All of this is mostly background and context for the changes here.
Many thanks to all the reviewers who helped here. Especially Sanjoy who caught several interesting bugs in the graph algorithms, David, Sean, and others who all helped with feedback.
Differential Revision: http://reviews.llvm.org/D21464
llvm-svn: 279618
show more ...
|
Revision tags: llvmorg-3.9.0-rc2 |
|
#
36e0d01e |
| 09-Aug-2016 |
Sean Silva <chisophugis@gmail.com> |
Consistently use FunctionAnalysisManager
Besides a general consistently benefit, the extra layer of indirection allows the mechanical part of https://reviews.llvm.org/D23256 that requires touching e
Consistently use FunctionAnalysisManager
Besides a general consistently benefit, the extra layer of indirection allows the mechanical part of https://reviews.llvm.org/D23256 that requires touching every transformation and analysis to be factored out cleanly.
Thanks to David for the suggestion.
llvm-svn: 278077
show more ...
|
Revision tags: llvmorg-3.9.0-rc1 |
|
#
6c138ce3 |
| 28-Jun-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Sink the module parsing from the fixture to the test as subsequent tests will want different IR.
Wanted this when writing tests for the proposed CG update stuff, and this is an easily separable
[PM] Sink the module parsing from the fixture to the test as subsequent tests will want different IR.
Wanted this when writing tests for the proposed CG update stuff, and this is an easily separable piece.
llvm-svn: 273973
show more ...
|
#
74a8a221 |
| 17-Jun-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Run clang-format over various parts of the new pass manager code prior to some very substantial patches to isolate any formatting-only changes.
llvm-svn: 272991
|
#
164a2aa6 |
| 17-Jun-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Remove support for omitting the AnalysisManager argument to new pass manager passes' `run` methods.
This removes a bunch of SFINAE goop from the pass manager and just requires pass authors to a
[PM] Remove support for omitting the AnalysisManager argument to new pass manager passes' `run` methods.
This removes a bunch of SFINAE goop from the pass manager and just requires pass authors to accept `AnalysisManager<IRUnitT> &` as a dead argument. This is a small price to pay for the simplicity of the system as a whole, despite the noise that changing it causes at this stage.
This will also helpfull allow us to make the signature of the run methods much more flexible for different kinds af passes to support things like intelligently updating the pass's progression over IR units.
While this touches many, many, files, the changes are really boring. Mostly made with the help of my trusty perl one liners.
Thanks to Sean and Hal for bouncing ideas for this with me in IRC.
llvm-svn: 272978
show more ...
|
Revision tags: llvmorg-3.8.1, llvmorg-3.8.1-rc1 |
|
#
03b42e41 |
| 14-Apr-2016 |
Mehdi Amini <mehdi.amini@apple.com> |
Remove every uses of getGlobalContext() in LLVM (but the C API)
At the same time, fixes InstructionsTest::CastInst unittest: yes you can leave the IR in an invalid state and exit when you don't dest
Remove every uses of getGlobalContext() in LLVM (but the C API)
At the same time, fixes InstructionsTest::CastInst unittest: yes you can leave the IR in an invalid state and exit when you don't destroy the context (like the global one), no longer now.
This is the first part of http://reviews.llvm.org/D19094
From: Mehdi Amini <mehdi.amini@apple.com> llvm-svn: 266379
show more ...
|
#
b47f8010 |
| 11-Mar-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Make the AnalysisManager parameter to run methods a reference.
This was originally a pointer to support pass managers which didn't use AnalysisManagers. However, that doesn't realistically come
[PM] Make the AnalysisManager parameter to run methods a reference.
This was originally a pointer to support pass managers which didn't use AnalysisManagers. However, that doesn't realistically come up much and the complexity of supporting it doesn't really make sense.
In fact, *many* parts of the pass manager were just assuming the pointer was never null already. This at least makes it much more explicit and clear.
llvm-svn: 263219
show more ...
|
Revision tags: llvmorg-3.8.0, llvmorg-3.8.0-rc3 |
|
#
c5d211ef |
| 23-Feb-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Remove an overly aggressive assert now that I can actually test the pattern that triggers it. This essentially requires an immutable function analysis, as that will survive anything we do to inv
[PM] Remove an overly aggressive assert now that I can actually test the pattern that triggers it. This essentially requires an immutable function analysis, as that will survive anything we do to invalidate it. When we have such patterns, the function analysis manager will not get cleared between runs of the proxy.
If we actually need an assert about how things are queried, we can add more elaborate machinery for computing it, but so far I'm not aware of significant value provided.
Thanks to Justin Lebar for noticing this when he made a (seemingly innocuous) change to FunctionAttrs that is enough to trigger it in one test there. Now it is covered by a direct test of the pass manager code.
llvm-svn: 261627
show more ...
|
#
74319922 |
| 23-Feb-2016 |
Chandler Carruth <chandlerc@gmail.com> |
[PM] Add a unittest for the CGSCC pass manager in the new pass manager system.
Previously, this was only being tested with larger integration tests. That makes it hard to isolated specific issues wi
[PM] Add a unittest for the CGSCC pass manager in the new pass manager system.
Previously, this was only being tested with larger integration tests. That makes it hard to isolated specific issues with it, and makes the APIs themselves less well tested. Add a unittest based around the same patterns used for testing the general pass manager.
llvm-svn: 261624
show more ...
|