SuccessChanges

Summary

  1. [EarlyCSE] Explicitly require AAResultsWrapperPass. (details)
  2. [lldb] Fix a crash when the ASTImporter is giving us two Imported callbacks for the same target decl (details)
  3. [Test] Add failing test for pr47457 (details)
  4. [lldb] Enable std::pair in CxxModuleHandler (details)
  5. [mlir] Added support for loops to BufferPlacement transformation. (details)
Commit 2bcc4db761768f1b7431237920f26360549ca268 by flo
[EarlyCSE] Explicitly require AAResultsWrapperPass.

The MemorySSAWrapperPass depends on AAResultsWrapperPass and if
MemorySSA is preserved but AAResultsWrapperPass is not, this could lead
to a crash when updating the last user of the MemorySSAWrapperPass.

Alternatively AAResultsWrapperPass could be marked preserved by GVN, but
I am not sure if that would be safe. I am not sure what is required in
order to preserve AAResultsWrapperPass. At the moment, it seems like a
couple of passes that do similar transforms to GVN are preserving it.

Reviewed By: asbirlea

Differential Revision: https://reviews.llvm.org/D87137
The file was modifiedllvm/lib/Transforms/Scalar/GVN.cpp (diff)
The file was addedllvm/test/Transforms/EarlyCSE/reuse-preserved-memoryssa.ll
The file was modifiedllvm/lib/Transforms/Scalar/EarlyCSE.cpp (diff)
Commit 7866b91405693df5b4cf6ba770b3a92d48b0c508 by Raphael Isemann
[lldb] Fix a crash when the ASTImporter is giving us two Imported callbacks for the same target decl

The ASTImporter has an `Imported(From, To)` callback that notifies subclasses
that a declaration has been imported in some way. LLDB uses this in the
`CompleteTagDeclsScope` to see which records have been imported into the scratch
context. If the record was declared inside the expression, then the
`CompleteTagDeclsScope` will forcibly import the full definition of that record
to the scratch context so that the expression AST can safely be disposed later
(otherwise we might end up going back to the deleted AST to complete the
minimally imported record). The way this is implemented is that there is a list
of decls that need to be imported (`m_decls_to_complete`) and we keep completing
the declarations inside that list until the list is empty. Every `To` Decl we
get via the `Imported` callback will be added to the list of Decls to be
completed.

There are some situations where the ASTImporter will actually give us two
`Imported` calls with the same `To` Decl. One way where this happens is if the
ASTImporter decides to merge an imported definition into an already imported
one. Another way is that the ASTImporter just happens to get two calls to
`ASTImporter::Import` for the same Decl. This for example happens when importing
the DeclContext of a Decl requires importing the Decl itself, such as when
importing a RecordDecl that was declared inside a function.

The bug addressed in this patch is that when we end up getting two `Imported`
calls for the same `To` Decl, then we would crash in the
`CompleteTagDeclsScope`.  That's because the first time we complete the Decl we
remove the Origin tracking information (that maps the Decl back to from where it
came from). The next time we try to complete the same `To` Decl the Origin
tracking information is gone and we hit the `to_context_md->getOrigin(decl).ctx
== m_src_ctx` assert (`getOrigin(decl).ctx` is a nullptr the second time as the
Origin was deleted).

This is actually a regression coming from D72495. Before D72495
`m_decls_to_complete` was actually a set so every declaration in there could
only be queued once to be completed. The set was changed to a vector to make the
iteration over it deterministic, but that also causes that we now potentially
end up trying to complete a Decl twice.

This patch essentially just reverts D72495 and makes the `CompleteTagDeclsScope`
use a SetVector for the list of declarations to be completed. The SetVector
should filter out the duplicates (as the original `set` did) and also ensure that
the completion order is deterministic. I actually couldn't find any way to cause
LLDB to reproduce this bug by merging declarations (this would require that we
for example declare two namespaces in a non-top-level expression which isn't
possible). But the bug reproduces very easily by just declaring a class in an
expression, so that's what the test is doing.

Reviewed By: shafik

Differential Revision: https://reviews.llvm.org/D85648
The file was addedlldb/test/API/lang/c/record_decl_in_expr/TestRecordDeclInExpr.py
The file was modifiedlldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp (diff)
Commit 37a7c0a00773f135d909eb9eba7f82547aee1e89 by mkazantsev
[Test] Add failing test for pr47457
The file was addedllvm/test/Transforms/LoopLoadElim/pr47457.ll
Commit b85222520f861a1812f991d6bd65950dda22f31b by Raphael Isemann
[lldb] Enable std::pair in CxxModuleHandler

This adds support for substituting std::pair instantiations with enabled
import-std-module.

With the fixes in parent revisions we can currently substitute a single pair
(however, a result that returns a second pair currently causes LLDB to crash
while importing the second template instantiation).

Reviewed By: aprantl

Differential Revision: https://reviews.llvm.org/D85141
The file was addedlldb/test/API/commands/expression/import-std-module/pair/TestPairFromStdModule.py
The file was modifiedlldb/source/Plugins/ExpressionParser/Clang/CxxModuleHandler.cpp (diff)
The file was addedlldb/test/API/commands/expression/import-std-module/pair/main.cpp
The file was addedlldb/test/API/commands/expression/import-std-module/pair/Makefile
Commit feb0b9c3bba7db6d547b552c3cdaa838559da664 by marcel.koester
[mlir] Added support for loops to BufferPlacement transformation.

The current BufferPlacement transformation cannot handle loops properly. Buffers
passed via backedges will not be freed automatically introducing memory leaks.
This CL adds support for loops to overcome these limitations.

Differential Revision: https://reviews.llvm.org/D85513
The file was modifiedmlir/test/Transforms/buffer-placement.mlir (diff)
The file was modifiedmlir/lib/Transforms/BufferPlacement.cpp (diff)