- Notifications
You must be signed in to change notification settings - Fork 15.3k
[NVPTX] Print SyncScope::ID if out of bounds #109871
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| @llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-backend-amdgpu Author: None (gonzalobg) ChangesFull diff: https://github.com/llvm/llvm-project/pull/109871.diff 7 Files Affected:
diff --git a/llvm/include/llvm/IR/LLVMContext.h b/llvm/include/llvm/IR/LLVMContext.h index 6ffa2bdaa319a7..8d60fb74a8aacc 100644 --- a/llvm/include/llvm/IR/LLVMContext.h +++ b/llvm/include/llvm/IR/LLVMContext.h @@ -130,6 +130,10 @@ class LLVMContext { /// scope names are ordered by increasing synchronization scope IDs. void getSyncScopeNames(SmallVectorImpl<StringRef> &SSNs) const; + /// getSyncScopeName - Returns the name of a SyncScope::ID + /// registered with LLVMContext, if any. + std::optional<StringRef> getSyncScopeName(SyncScope::ID Id) const; + /// Define the GC for a function void setGC(const Function &Fn, std::string GCName); diff --git a/llvm/lib/IR/LLVMContext.cpp b/llvm/lib/IR/LLVMContext.cpp index c0fee93a233808..e897cd2c9fd858 100644 --- a/llvm/lib/IR/LLVMContext.cpp +++ b/llvm/lib/IR/LLVMContext.cpp @@ -330,6 +330,10 @@ void LLVMContext::getSyncScopeNames(SmallVectorImpl<StringRef> &SSNs) const { pImpl->getSyncScopeNames(SSNs); } +std::optional<StringRef> LLVMContext::getSyncScopeName(SyncScope::ID Id) const { + return pImpl->getSyncScopeName(Id); +} + void LLVMContext::setGC(const Function &Fn, std::string GCName) { pImpl->GCNames[&Fn] = std::move(GCName); } diff --git a/llvm/lib/IR/LLVMContextImpl.cpp b/llvm/lib/IR/LLVMContextImpl.cpp index 4f1ef8cec32133..f2c965a45df3ae 100644 --- a/llvm/lib/IR/LLVMContextImpl.cpp +++ b/llvm/lib/IR/LLVMContextImpl.cpp @@ -244,6 +244,16 @@ void LLVMContextImpl::getSyncScopeNames( SSNs[SSE.second] = SSE.first(); } +std::optional<StringRef> +LLVMContextImpl::getSyncScopeName(SyncScope::ID Id) const { + for (const auto &SSE : SSC) { + if (SSE.second != Id) + continue; + return SSE.first(); + } + return std::nullopt; +} + /// Gets the OptPassGate for this LLVMContextImpl, which defaults to the /// singleton OptBisect if not explicitly set. OptPassGate &LLVMContextImpl::getOptPassGate() const { diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h index e76f004b590efe..971091f3040614 100644 --- a/llvm/lib/IR/LLVMContextImpl.h +++ b/llvm/lib/IR/LLVMContextImpl.h @@ -1665,6 +1665,10 @@ class LLVMContextImpl { /// scope names are ordered by increasing synchronization scope IDs. void getSyncScopeNames(SmallVectorImpl<StringRef> &SSNs) const; + /// getSyncScopeName - Returns the name of a SyncScope::ID + /// registered with LLVMContext, if any. + std::optional<StringRef> getSyncScopeName(SyncScope::ID Id) const; + /// Maintain the GC name for each function. /// /// This saves allocating an additional word in Function for programs which diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp index a9754ba357893f..3e330cb7aee780 100644 --- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp +++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp @@ -16142,11 +16142,8 @@ static bool atomicIgnoresDenormalModeOrFPModeIsFTZ(const AtomicRMWInst *RMW) { static OptimizationRemark emitAtomicRMWLegalRemark(const AtomicRMWInst *RMW) { LLVMContext &Ctx = RMW->getContext(); - SmallVector<StringRef> SSNs; - Ctx.getSyncScopeNames(SSNs); - StringRef MemScope = SSNs[RMW->getSyncScopeID()].empty() - ? "system" - : SSNs[RMW->getSyncScopeID()]; + StringRef SS = Ctx.getSyncScopeName(RMW->getSyncScopeID()).value_or(""); + StringRef MemScope = SS.empty() ? StringRef("system") : SS; return OptimizationRemark(DEBUG_TYPE, "Passed", RMW) << "Hardware instruction generated for atomic " diff --git a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp index 56c96ea943b89d..7ce4c2e9daf6c7 100644 --- a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp +++ b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp @@ -4176,7 +4176,7 @@ bool NVPTXDAGToDAGISel::tryFence(SDNode *N) { return true; } -NVPTXScopes::NVPTXScopes(LLVMContext &C) { +NVPTXScopes::NVPTXScopes(LLVMContext &C) : CTX(&C) { Scopes[C.getOrInsertSyncScopeID("singlethread")] = NVPTX::Scope::Thread; Scopes[C.getOrInsertSyncScopeID("")] = NVPTX::Scope::System; Scopes[C.getOrInsertSyncScopeID("block")] = NVPTX::Scope::Block; @@ -4190,13 +4190,11 @@ NVPTX::Scope NVPTXScopes::operator[](SyncScope::ID ID) const { "NVPTXScopes::operator[]"); auto S = Scopes.find(ID); - if (S == Scopes.end()) { - // TODO: - // - Add API to LLVMContext to get the name of a single scope. - // - Use that API here to print an error containing the name - // of this Unknown ID. - report_fatal_error(formatv("Could not find scope ID={}.", int(ID))); - } + if (S == Scopes.end()) + report_fatal_error(formatv("Could not find scope ID={} with name \"{}\".", + int(ID), + CTX->getSyncScopeName(ID).value_or("unknown"))); + return S->second; } diff --git a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h index c128c082c29837..f925fc67fbccb7 100644 --- a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h +++ b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h @@ -35,6 +35,7 @@ struct NVPTXScopes { private: SmallMapVector<SyncScope::ID, NVPTX::Scope, 8> Scopes{}; + LLVMContext *CTX = nullptr; }; class LLVM_LIBRARY_VISIBILITY NVPTXDAGToDAGISel : public SelectionDAGISel { |
| | ||
| private: | ||
| SmallMapVector<SyncScope::ID, NVPTX::Scope, 8> Scopes{}; | ||
| LLVMContext *CTX = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CTX appears to be required and should be a reference.
| for (const auto &SSE : SSC) { | ||
| if (SSE.second != Id) | ||
| continue; | ||
| return SSE.first(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for explicit continue here:
for (const auto &SSE : SSC) if (SSE.second == Id) return SSE.first();
No description provided.