Skip to content
This repository was archived by the owner on Mar 2, 2026. It is now read-only.

[SYCL] Improve record and replay edge determination#46

Merged
Bensuo merged 5 commits intosycl-graph-record-replayfrom
ben/record-edges
Dec 12, 2022
Merged

[SYCL] Improve record and replay edge determination#46
Bensuo merged 5 commits intosycl-graph-record-replayfrom
ben/record-edges

Conversation

@Bensuo
Copy link
Collaborator

@Bensuo Bensuo commented Dec 6, 2022

Improves the way that edges are detected in recorded graph nodes so that now edges should be correctly determined. Currently only tested with pointer edges (for USM pointers).

Also adds a buffer based dotp example using record and replay but this currently throws with the same error as #24.

- Improved method of determining edges for nodes recorded to a graph.
- Currently only considers pointer args.
- Copies of args are now stored inside `node_impl`
- Remove last node mechanism needed for previous simple approach.
@Bensuo Bensuo requested review from EwanC and reble December 6, 2022 13:16
@EwanC EwanC added the Graph Implementation Related to DPC++ implementation and testing label Dec 8, 2022
@Bensuo Bensuo merged this pull request into sycl-graph-record-replay Dec 12, 2022
@Bensuo Bensuo deleted the ben/record-edges branch December 12, 2022 11:55
reble pushed a commit that referenced this pull request May 15, 2023
…callback

The `TypeSystemMap::m_mutex` guards against concurrent modifications
of members of `TypeSystemMap`. In particular, `m_map`.

`TypeSystemMap::ForEach` iterates through the entire `m_map` calling
a user-specified callback for each entry. This is all done while
`m_mutex` is locked. However, there's nothing that guarantees that
the callback itself won't call back into `TypeSystemMap` APIs on the
same thread. This lead to double-locking `m_mutex`, which is undefined
behaviour. We've seen this cause a deadlock in the swift plugin with
following backtrace:

```

int main() {
    std::unique_ptr<int> up = std::make_unique<int>(5);

    volatile int val = *up;
    return val;
}

clang++ -std=c++2a -g -O1 main.cpp

./bin/lldb -o “br se -p return” -o run -o “v *up” -o “expr *up” -b
```

```
frame #4: std::lock_guard<std::mutex>::lock_guard
frame #5: lldb_private::TypeSystemMap::GetTypeSystemForLanguage <<<< Lock #2
frame #6: lldb_private::TypeSystemMap::GetTypeSystemForLanguage
frame #7: lldb_private::Target::GetScratchTypeSystemForLanguage
...
frame #26: lldb_private::SwiftASTContext::LoadLibraryUsingPaths
frame #27: lldb_private::SwiftASTContext::LoadModule
frame #30: swift::ModuleDecl::collectLinkLibraries
frame #31: lldb_private::SwiftASTContext::LoadModule
frame #34: lldb_private::SwiftASTContext::GetCompileUnitImportsImpl
frame #35: lldb_private::SwiftASTContext::PerformCompileUnitImports
frame #36: lldb_private::TypeSystemSwiftTypeRefForExpressions::GetSwiftASTContext
frame #37: lldb_private::TypeSystemSwiftTypeRefForExpressions::GetPersistentExpressionState
frame #38: lldb_private::Target::GetPersistentSymbol
frame #41: lldb_private::TypeSystemMap::ForEach                 <<<< Lock #1
frame #42: lldb_private::Target::GetPersistentSymbol
frame #43: lldb_private::IRExecutionUnit::FindInUserDefinedSymbols
frame #44: lldb_private::IRExecutionUnit::FindSymbol
frame #45: lldb_private::IRExecutionUnit::MemoryManager::GetSymbolAddressAndPresence
frame #46: lldb_private::IRExecutionUnit::MemoryManager::findSymbol
frame #47: non-virtual thunk to lldb_private::IRExecutionUnit::MemoryManager::findSymbol
frame #48: llvm::LinkingSymbolResolver::findSymbol
frame #49: llvm::LegacyJITSymbolResolver::lookup
frame #50: llvm::RuntimeDyldImpl::resolveExternalSymbols
frame #51: llvm::RuntimeDyldImpl::resolveRelocations
frame #52: llvm::MCJIT::finalizeLoadedModules
frame #53: llvm::MCJIT::finalizeObject
frame #54: lldb_private::IRExecutionUnit::ReportAllocations
frame #55: lldb_private::IRExecutionUnit::GetRunnableInfo
frame #56: lldb_private::ClangExpressionParser::PrepareForExecution
frame #57: lldb_private::ClangUserExpression::TryParse
frame #58: lldb_private::ClangUserExpression::Parse
```

Our solution is to simply iterate over a local copy of `m_map`.

**Testing**

* Confirmed on manual reproducer (would reproduce 100% of the time
  before the patch)

Differential Revision: https://reviews.llvm.org/D149949
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Graph Implementation Related to DPC++ implementation and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants