[Offload] Check for initialization

[RossBrunton]

All entry points (except olInit) now check that offload has been
initialized. If not, a new OL_ERRC_UNINITIALIZED error is returned.

[llvmbot]

@llvm/pr-subscribers-offload

Author: Ross Brunton (RossBrunton)

Changes

All entry points (except olInit) now check that offload has been
initialized. If not, a new OL_ERRC_UNINITIALIZED error is returned.

---

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

5 Files Affected:

• (modified) offload/liboffload/API/Common.td (+1)
• (modified) offload/liboffload/include/OffloadImpl.hpp (+1)
• (modified) offload/liboffload/src/OffloadImpl.cpp (+2)
• (modified) offload/liboffload/src/OffloadLib.cpp (+5)
• (modified) offload/tools/offload-tblgen/EntryPointGen.cpp (+12)

diff --git a/offload/liboffload/API/Common.td b/offload/liboffload/API/Common.td
index 8a2ecd6c6e8f4..cd8c3c63fde81 100644
--- a/offload/liboffload/API/Common.td
+++ b/offload/liboffload/API/Common.td
@@ -106,6 +106,7 @@ def ErrorCode : Enum {
     Etor<"ASSEMBLE_FAILURE", "assembler failure while processing binary image">,
     Etor<"LINK_FAILURE", "linker failure while processing binary image">,
     Etor<"BACKEND_FAILURE", "the plugin backend is in an invalid or unsupported state">,
+    Etor<"UNINITIALIZED", "not initialized">,
 
     // Handle related errors - only makes sense for liboffload
     Etor<"INVALID_NULL_HANDLE", "a handle argument is null when it should not be">,
diff --git a/offload/liboffload/include/OffloadImpl.hpp b/offload/liboffload/include/OffloadImpl.hpp
index 9b0a21cb9ae12..4349ac3184da9 100644
--- a/offload/liboffload/include/OffloadImpl.hpp
+++ b/offload/liboffload/include/OffloadImpl.hpp
@@ -27,6 +27,7 @@ struct OffloadConfig {
   bool ValidationEnabled = true;
 };
 
+bool &offloadInited();
 OffloadConfig &offloadConfig();
 
 // Use the StringSet container to efficiently deduplicate repeated error
diff --git a/offload/liboffload/src/OffloadImpl.cpp b/offload/liboffload/src/OffloadImpl.cpp
index 770c212d804d2..cab9f3b5535f5 100644
--- a/offload/liboffload/src/OffloadImpl.cpp
+++ b/offload/liboffload/src/OffloadImpl.cpp
@@ -174,6 +174,8 @@ Error olInit_impl() {
   static std::once_flag InitFlag;
   std::call_once(InitFlag, initPlugins);
 
+  offloadInited() = true;
+
   return Error::success();
 }
 Error olShutDown_impl() { return Error::success(); }
diff --git a/offload/liboffload/src/OffloadLib.cpp b/offload/liboffload/src/OffloadLib.cpp
index 8662d3a44124b..febe1de2cc507 100644
--- a/offload/liboffload/src/OffloadLib.cpp
+++ b/offload/liboffload/src/OffloadLib.cpp
@@ -35,6 +35,11 @@ OffloadConfig &offloadConfig() {
   return Config;
 }
 
+bool &offloadInited() {
+  static bool OffloadInitedFlag{false};
+  return OffloadInitedFlag;
+}
+
 namespace llvm {
 namespace offload {
 // Pull in the declarations for the implementation functions. The actual entry
diff --git a/offload/tools/offload-tblgen/EntryPointGen.cpp b/offload/tools/offload-tblgen/EntryPointGen.cpp
index 85c5c50bf2f20..b9cd7d030ef4f 100644
--- a/offload/tools/offload-tblgen/EntryPointGen.cpp
+++ b/offload/tools/offload-tblgen/EntryPointGen.cpp
@@ -73,6 +73,10 @@ static void EmitEntryPointFunc(const FunctionRec &F, raw_ostream &OS) {
   }
   OS << ") {\n";
 
+  // Check offload is inited
+  if (F.getName() != "olInit")
+    OS << "if (!offloadInited()) return &UninitError;";
+
   // Emit pre-call prints
   OS << TAB_1 "if (offloadConfig().TracingEnabled) {\n";
   OS << formatv(TAB_2 "llvm::errs() << \"---> {0}\";\n", F.getName());
@@ -134,6 +138,14 @@ static void EmitCodeLocWrapper(const FunctionRec &F, raw_ostream &OS) {
 
 void EmitOffloadEntryPoints(const RecordKeeper &Records, raw_ostream &OS) {
   OS << GenericHeader;
+
+  constexpr const char *UninitMessage =
+      "liboffload has not been initialized - please call olInit before using "
+      "this API";
+  OS << formatv("static {0}_error_struct_t UninitError = "
+                "{{{1}_ERRC_UNINITIALIZED, \"{2}\"};",
+                PrefixLower, PrefixUpper, UninitMessage);
+
   for (auto *R : Records.getAllDerivedDefinitions("Function")) {
     EmitValidationFunc(FunctionRec{R}, OS);
     EmitEntryPointFunc(FunctionRec{R}, OS);

[jhuber6]

Realistically we should have one big pointer for all our global state and this just returns whether or not that's null.

[RossBrunton]

I agree. I was thinking that this would end up being a field inside that struct but a null check makes more sense. I'll work on that tomorrow and rebase this on top of it.