-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[CIR] Change unreachable to diagnostic for ill-equipped clang #152614
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
CLANG_ENABLE_CIR Add a new frontend diagnostic that informs users with ill-equipped clang binaries that they need to rebuild with -DCLANG_ENABLE_CIR=ON. This requires adding some plumbing for the new `cir-support` lit option. This improves the user experience as they may have a clang binary that wasn't built with MLIR/CIR support and using `-emit-cir` results in an unfriendly crash.
@llvm/pr-subscribers-clang Author: Justin Stitt (JustinStitt) ChangesUsing a clang binary that wasn't built with MLIR/CIR support and using Add a new frontend diagnostic that informs users with ill-equipped clang binaries that they need to rebuild with Getting a worthwhile test involves adding some plumbing for a new As an aside, does Clang have any methods of hiding features in Full diff: https://github.com/llvm/llvm-project/pull/152614.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 8a8db27490f06..89bca11540147 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -132,6 +132,8 @@ def err_fe_no_pch_in_dir : Error<
"no suitable precompiled header file found in directory '%0'">;
def err_fe_action_not_available : Error<
"action %0 not compiled in">;
+def err_fe_cir_not_built : Error<"clang IR support not available, rebuild "
+ "clang with -DCLANG_ENABLE_CIR=ON">;
def err_fe_invalid_multiple_actions : Error<
"'%0' action ignored; '%1' action specified previously">;
def err_fe_invalid_alignment : Error<
diff --git a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
index 443eb4f1a29bf..67372a37c05f7 100644
--- a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -81,7 +81,8 @@ CreateFrontendBaseAction(CompilerInstance &CI) {
#if CLANG_ENABLE_CIR
return std::make_unique<cir::EmitCIRAction>();
#else
- llvm_unreachable("CIR suppport not built into clang");
+ CI.getDiagnostics().Report(diag::err_fe_cir_not_built);
+ return nullptr;
#endif
case EmitHTML: return std::make_unique<HTMLPrintAction>();
case EmitLLVM: {
diff --git a/clang/test/Frontend/cir-not-built.c b/clang/test/Frontend/cir-not-built.c
new file mode 100644
index 0000000000000..f9af2c4193b28
--- /dev/null
+++ b/clang/test/Frontend/cir-not-built.c
@@ -0,0 +1,12 @@
+// Test that using -emit-cir when clang is not built with CIR support gives a proper error message
+// instead of crashing.
+
+// This test should only run when CIR support is NOT enabled
+// REQUIRES: !cir-support
+
+// RUN: not %clang_cc1 -emit-cir %s 2>&1 | FileCheck %s
+// CHECK: error: clang IR support not available, rebuild clang with -DCLANG_ENABLE_CIR=ON
+
+int main(void) {
+ return 0;
+}
diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index 1957bb1715eb6..4b2ecb0a6ca4c 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -224,6 +224,10 @@ def have_host_clang_repl_cuda():
)
)
+# ClangIR support
+if config.clang_enable_cir:
+ config.available_features.add("cir-support")
+
llvm_config.add_tool_substitutions(tools, tool_dirs)
config.substitutions.append(
|
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.
Looks reasonable to me. I have little experience in the frontend configuration code, so I'll let someone else to chip in.
Using a clang binary that wasn't built with MLIR/CIR support and using
-emit-cir
results in an unfriendly crash. We really shouldn't be abusing unreachables to inform users of mis-configurations.Add a new frontend diagnostic that informs users with ill-equipped clang binaries that they need to rebuild with
-DCLANG_ENABLE_CIR=ON
to use-emit-cir
. This matches how the frontend action diagnostics behave inExecuteCompilerInvocation.cpp
witherr_fe_action_not_available
.Getting a worthwhile test involves adding some plumbing for a new
cir-support
lit option.As an aside, does Clang have any methods of hiding features in
--help
when they are not compiled in? It might be a good idea to hide-emit-cir
from the help text in this case.