Skip to content

Conversation

@gonzalobg
Copy link
Contributor

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2024

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-backend-nvptx

@llvm/pr-subscribers-backend-amdgpu

Author: None (gonzalobg)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/109871.diff

7 Files Affected:

  • (modified) llvm/include/llvm/IR/LLVMContext.h (+4)
  • (modified) llvm/lib/IR/LLVMContext.cpp (+4)
  • (modified) llvm/lib/IR/LLVMContextImpl.cpp (+10)
  • (modified) llvm/lib/IR/LLVMContextImpl.h (+4)
  • (modified) llvm/lib/Target/AMDGPU/SIISelLowering.cpp (+2-5)
  • (modified) llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp (+6-8)
  • (modified) llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h (+1)
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;
Copy link
Member

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.

Comment on lines +249 to +253
for (const auto &SSE : SSC) {
if (SSE.second != Id)
continue;
return SSE.first();
}
Copy link
Member

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(); 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment