[NVPTX] Add more clear error message for using invalid syncscope #165737
Conversation
|
@llvm/pr-subscribers-backend-nvptx Author: Stefan Mada (smada3) ChangesUsing invalid syncscopes on certain NVVM intrinsics causes an obscure error to appear: (error 9: NVVM_ERROR_COMPILATION), libNVVM extra log: Could not find scope ID=5. This is not a very helpful error. A much more useful error would be something like 'NVPTX does not support syncscope "agent"' This would immediately make it clear that the issue is not NVPTX specific, but actually from code being fed to NVPTX. This would save users time in debugging issues related to this. Full diff: https://github.com/llvm/llvm-project/pull/165737.diff 3 Files Affected:
diff --git a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
index 7e7ee754c250d..0bcb9c89b3e8b 100644
--- a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
@@ -1836,7 +1836,7 @@ bool NVPTXDAGToDAGISel::tryFence(SDNode *N) {
return true;
}
-NVPTXScopes::NVPTXScopes(LLVMContext &C) {
+NVPTXScopes::NVPTXScopes(LLVMContext &C) : Context(&C) {
Scopes[C.getOrInsertSyncScopeID("singlethread")] = NVPTX::Scope::Thread;
Scopes[C.getOrInsertSyncScopeID("")] = NVPTX::Scope::System;
Scopes[C.getOrInsertSyncScopeID("block")] = NVPTX::Scope::Block;
@@ -1851,11 +1851,24 @@ NVPTX::Scope NVPTXScopes::operator[](SyncScope::ID ID) const {
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)));
+ // Get the actual scope name from LLVMContext for a better error message
+ std::string scopeName = "<unknown>";
+ if (auto name = Context->getSyncScopeName(ID))
+ scopeName = name->str();
+
+ // Build list of supported syncscopes programmatically
+ std::string supportedScopes;
+ for (const auto &Entry : Scopes) {
+ if (!supportedScopes.empty())
+ supportedScopes += ", ";
+ if (auto name = Context->getSyncScopeName(Entry.first))
+ supportedScopes += name->empty() ? "<empty string>" : name->str();
+ }
+
+ reportFatalUsageError(
+ formatv("NVPTX backend does not support syncscope \"{0}\" (ID={1}).\n"
+ "Supported syncscopes are: {2}.",
+ scopeName, int(ID), supportedScopes));
}
return S->second;
}
diff --git a/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h b/llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.h
index c912e709d0aa0..5fb1267224bd5 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 *Context = nullptr;
};
class LLVM_LIBRARY_VISIBILITY NVPTXDAGToDAGISel : public SelectionDAGISel {
diff --git a/llvm/test/CodeGen/NVPTX/cmpxchg-unsupported-syncscope.err.ll b/llvm/test/CodeGen/NVPTX/cmpxchg-unsupported-syncscope.err.ll
new file mode 100644
index 0000000000000..d3853e2fdaa88
--- /dev/null
+++ b/llvm/test/CodeGen/NVPTX/cmpxchg-unsupported-syncscope.err.ll
@@ -0,0 +1,11 @@
+; RUN: not llc -mcpu=sm_100a -mtriple=nvptx64 -mattr=+ptx86 %s 2>&1 | FileCheck %s
+
+; Test that we get a clear error message when using an unsupported syncscope.
+
+; CHECK: NVPTX backend does not support syncscope "agent"
+; CHECK: Supported syncscopes are: singlethread, <empty string>, block, cluster, device
+define i32 @cmpxchg_unsupported_syncscope_agent(ptr %addr, i32 %cmp, i32 %new) {
+ %result = cmpxchg ptr %addr, i32 %cmp, i32 %new syncscope("agent") monotonic monotonic
+ %value = extractvalue { i32, i1 } %result, 0
+ ret i32 %value
+}
|
| supportedScopes += ", "; | ||
| if (auto name = Context->getSyncScopeName(Entry.first)) | ||
| supportedScopes += name->empty() ? "<empty string>" : name->str(); | ||
| } |
There was a problem hiding this comment.
The error-reporting enhancement LGTM.
Would collecting the scopes in a small-vector and then using llvm::join (with comma as separator) make it simpler?
There was a problem hiding this comment.
Agreed, better yet use StringRefs as well and just comma join the list as part of formatv https://llvm.org/docs/ProgrammersManual.html#formatv-examples
There was a problem hiding this comment.
Nice, didn't know llvm:join existed. Changed to use both, take a look when you get a chance.
There was a problem hiding this comment.
You don't even need to use join in this case, just use the existing formatv support for ranges
| // of this Unknown ID. | ||
| report_fatal_error(formatv("Could not find scope ID={}.", int(ID))); | ||
| // Get the actual scope name from LLVMContext for a better error message | ||
| std::string scopeName = "<unknown>"; |
There was a problem hiding this comment.
Is this expected to ever happen? If so, please add a test, if not perhaps change to an assert?
4 participants
Using invalid syncscopes on certain NVVM intrinsics causes an obscure error to appear: (error 9: NVVM_ERROR_COMPILATION), libNVVM extra log: Could not find scope ID=5.
This is not a very helpful error. A much more useful error would be something like 'NVPTX does not support syncscope "agent"'
This would immediately make it clear that the issue is not NVPTX specific, but actually from code being fed to NVPTX. This would save users time in debugging issues related to this.