I think that clang/cmake/caches/README.txt should be updated too, with a description of how to use it (which would be more permanent than the revision summary).

Rebase to fix test failures.

I think that Extensions.rst should be updated as well, including a description of the versioning scheme, the fact that the section name is now a semantically significant property, and a description of both the v0 and v1 formats.

In D79978#2145979, @MaskRay wrote:

I cannot find a test with .cfa_offset or describing a non-rbp register, so the implementation is under-tested. How about leveraging inline asm to clobber callee-saved registers?

In D79978#2145063, @MaskRay wrote:

In D79978#2141992, @wmi wrote:

In D79978#2141959, @MaskRay wrote:

Ack. Then what instructions should be placed at the top of these basic blocks? Should .cfi_def_cfa_register %rbp be placed as well? If you move these basic blocks around, .cfi_def_cfa_register %rbp is currently not tracked.

That is because .cfi_def_cfa %rbp, 16 is identical to the following:
.cfi_def_cfa_register %rbp
.cfi_def_cfa_offset 16

Honestly I am not a CFI expert but I have read enough bits of LLVM libunwind and am not completely CFI illiterate (I have fixed a very subtle negative cfiDefCfa bug). The description of the patch is still puzzling me.
I think it lacks a summary about what the patch intends to do.

Is the intention: if the entry block stays in the front of the function, while other basic blocks can be randomly shuffled => the CFI states of all basic blocks are the same no matter how non-entry basic blocks are shuffled?

Or

The entry basic block can be shuffled as well?

We explicitly want to support the case where all BBs can be shuffled around arbitrarily.

Updated the patch to manually merge vectors of large GEP offsets when doing a RAUW of an invariant.group strip/launder intrinsic. This requires only handling keys and not values of the map, as values are always GetElementPtr instructions and not intrinsic calls.

The other AssertingVHs held by large GEP splitting code are either AssertingVH<GetElementPtrInst> or refer to possibly bitcasted GetElementPtrInsts. As far as I can see, there is no RAUWing of those being done in CGP at the moment.

Looks good to me. Obviously, you should wait for someone more competent than me to accept it, too.

Otherwise looks good to me. Obviously, you should wait for someone more competent than me to approve it ;)

I think that MarkVTableUsed should be called somewhere in Sema (possibly ActOnFinishCXXMemberDecls?) if ForceEmitVTables is on. This probably requires making ForceEmitVTables a LangOption in addition to it being a CodeGenOption.

This is not sound: sometimes the forcefully emitted vtable is incorrect due to destructor aliasing. This happens e.g. in the Bullet benchmark from the llvm test suite. A simplified example follows.

Of course, you should wait for an LGTM from someone more competent than me ;)

Removed unnecessary { }.

Added a direct AA test and cautionary comments in both BasicAA and CaptureTracking, removed unnecessary { }.

Otherwise looks good to me.

Looks good to me.

I think it would be a little bit cleaner if the relevant checks were added to verifyRemoved too, where all other maps are traversed in debug builds. However, AssertingVH is probably a good enough guarantee ;)

Would handling NonLocalDefsCache in the same places as other DenseMaps from Instruction * be sufficient? In particular, RemoveCachedNonLocalPointerDependencies, called by both removeInstruction and invalidateCachedPointerInfo, looks promising ;)

I am not sure that ValueMap can be moveable: if it was moved, the ValueMapT *Map pointer in ValueMapCallbackVH could become invalid.