diff options
author | Johannes Doerfert <johannes@jdoerfert.de> | 2023-12-01 14:47:00 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-12-01 14:47:00 -0800 |
commit | 5fe741f08e09a2eca276fc11d39240caa3d23cb2 (patch) | |
tree | 4bde195db7c3f1af2ecab8bcef5913da3362dc1b | |
parent | 75867f8e4a9a06df3b2cafe662d13ee78bb7fc67 (diff) |
[OpenMP] Separate Requirements into a standalone header (#74126)
This is not completely NFC since we now check all 4 requirements and the
test is checking the good and the bad case for combining flags.
-rw-r--r-- | openmp/libomptarget/include/PluginManager.h | 16 | ||||
-rw-r--r-- | openmp/libomptarget/include/Shared/Requirements.h | 88 | ||||
-rw-r--r-- | openmp/libomptarget/include/omptarget.h | 15 | ||||
-rw-r--r-- | openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h | 1 | ||||
-rw-r--r-- | openmp/libomptarget/src/device.cpp | 6 | ||||
-rw-r--r-- | openmp/libomptarget/src/interface.cpp | 2 | ||||
-rw-r--r-- | openmp/libomptarget/src/rtl.cpp | 41 | ||||
-rw-r--r-- | openmp/libomptarget/test/offloading/requires.c | 20 |
8 files changed, 118 insertions, 71 deletions
diff --git a/openmp/libomptarget/include/PluginManager.h b/openmp/libomptarget/include/PluginManager.h index 91298928716b..5c7e13739664 100644 --- a/openmp/libomptarget/include/PluginManager.h +++ b/openmp/libomptarget/include/PluginManager.h @@ -15,6 +15,7 @@ #include "Shared/APITypes.h" #include "Shared/PluginAPI.h" +#include "Shared/Requirements.h" #include "device.h" @@ -23,6 +24,7 @@ #include "llvm/ADT/iterator_range.h" #include "llvm/Support/DynamicLibrary.h" +#include <cstdint> #include <list> #include <mutex> #include <string> @@ -66,13 +68,8 @@ struct PluginAdaptorTy { /// RTLs identified in the system. struct PluginAdaptorManagerTy { - int64_t RequiresFlags = OMP_REQ_UNDEFINED; - explicit PluginAdaptorManagerTy() = default; - // Register the clauses of the requires directive. - void registerRequires(int64_t Flags); - // Register a shared library with all (compatible) RTLs. void registerLib(__tgt_bin_desc *Desc); @@ -148,12 +145,21 @@ struct PluginManager { return llvm::make_range(PluginAdaptors.begin(), PluginAdaptors.end()); } + /// Return the user provided requirements. + int64_t getRequirements() const { return Requirements.getRequirements(); } + + /// Add \p Flags to the user provided requirements. + void addRequirements(int64_t Flags) { Requirements.addRequirements(Flags); } + private: bool RTLsLoaded = false; llvm::SmallVector<__tgt_bin_desc *> DelayedBinDesc; // List of all plugin adaptors, in use or not. std::list<PluginAdaptorTy> PluginAdaptors; + + /// The user provided requirements. + RequirementCollection Requirements; }; extern PluginManager *PM; diff --git a/openmp/libomptarget/include/Shared/Requirements.h b/openmp/libomptarget/include/Shared/Requirements.h new file mode 100644 index 000000000000..19d6b8ffca49 --- /dev/null +++ b/openmp/libomptarget/include/Shared/Requirements.h @@ -0,0 +1,88 @@ +//===-- OpenMP/Requirements.h - User required requirements -----*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Handling of the `omp requires` directive, e.g., requiring unified shared +// memory. +// +//===----------------------------------------------------------------------===// + +#ifndef OMPTARGET_OPENMP_REQUIREMENTS_H +#define OMPTARGET_OPENMP_REQUIREMENTS_H + +#include "Shared/Debug.h" + +#include "llvm/ADT/StringRef.h" + +#include <cassert> +#include <cstdint> + +enum OpenMPOffloadingRequiresDirFlags : int64_t { + /// flag undefined. + OMP_REQ_UNDEFINED = 0x000, + /// no requires directive present. + OMP_REQ_NONE = 0x001, + /// reverse_offload clause. + OMP_REQ_REVERSE_OFFLOAD = 0x002, + /// unified_address clause. + OMP_REQ_UNIFIED_ADDRESS = 0x004, + /// unified_shared_memory clause. + OMP_REQ_UNIFIED_SHARED_MEMORY = 0x008, + /// dynamic_allocators clause. + OMP_REQ_DYNAMIC_ALLOCATORS = 0x010 +}; + +class RequirementCollection { + int64_t SetFlags = OMP_REQ_UNDEFINED; + + /// Check consistency between different requires flags (from different + /// translation units). + void checkConsistency(int64_t NewFlags, int64_t SetFlags, + OpenMPOffloadingRequiresDirFlags Flag, + llvm::StringRef Clause) { + if ((SetFlags & Flag) != (NewFlags & Flag)) { + FATAL_MESSAGE(2, "'#pragma omp requires %s' not used consistently!", + Clause.data()); + } + } + +public: + /// Register \p NewFlags as part of the user requirements. + void addRequirements(int64_t NewFlags) { + // TODO: add more elaborate check. + // Minimal check: only set requires flags if previous value + // is undefined. This ensures that only the first call to this + // function will set the requires flags. All subsequent calls + // will be checked for compatibility. + assert(NewFlags != OMP_REQ_UNDEFINED && + "illegal undefined flag for requires directive!"); + if (SetFlags == OMP_REQ_UNDEFINED) { + SetFlags = NewFlags; + return; + } + + // If multiple compilation units are present enforce + // consistency across all of them for require clauses: + // - reverse_offload + // - unified_address + // - unified_shared_memory + // - dynamic_allocators + checkConsistency(NewFlags, SetFlags, OMP_REQ_REVERSE_OFFLOAD, + "reverse_offload"); + checkConsistency(NewFlags, SetFlags, OMP_REQ_UNIFIED_ADDRESS, + "unified_address"); + checkConsistency(NewFlags, SetFlags, OMP_REQ_UNIFIED_SHARED_MEMORY, + "unified_shared_memory"); + checkConsistency(NewFlags, SetFlags, OMP_REQ_DYNAMIC_ALLOCATORS, + "dynamic_allocators"); + } + + /// Return the user provided requirements. + int64_t getRequirements() const { return SetFlags; } +}; + +#endif // OMPTARGET_OPENMP_DEVICE_REQUIREMENTS_H diff --git a/openmp/libomptarget/include/omptarget.h b/openmp/libomptarget/include/omptarget.h index 829e000f4fb3..476a158019d3 100644 --- a/openmp/libomptarget/include/omptarget.h +++ b/openmp/libomptarget/include/omptarget.h @@ -99,21 +99,6 @@ enum OpenMPOffloadingDeclareTargetFlags { OMP_DECLARE_TARGET_INDIRECT = 0x08 }; -enum OpenMPOffloadingRequiresDirFlags { - /// flag undefined. - OMP_REQ_UNDEFINED = 0x000, - /// no requires directive present. - OMP_REQ_NONE = 0x001, - /// reverse_offload clause. - OMP_REQ_REVERSE_OFFLOAD = 0x002, - /// unified_address clause. - OMP_REQ_UNIFIED_ADDRESS = 0x004, - /// unified_shared_memory clause. - OMP_REQ_UNIFIED_SHARED_MEMORY = 0x008, - /// dynamic_allocators clause. - OMP_REQ_DYNAMIC_ALLOCATORS = 0x010 -}; - enum TargetAllocTy : int32_t { TARGET_ALLOC_DEVICE = 0, TARGET_ALLOC_HOST, diff --git a/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h b/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h index dd601e6b4231..ab6c457fba78 100644 --- a/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h +++ b/openmp/libomptarget/plugins-nextgen/common/include/PluginInterface.h @@ -22,6 +22,7 @@ #include "Shared/Debug.h" #include "Shared/Environment.h" #include "Shared/EnvironmentVar.h" +#include "Shared/Requirements.h" #include "Shared/Utils.h" #include "GlobalHandler.h" diff --git a/openmp/libomptarget/src/device.cpp b/openmp/libomptarget/src/device.cpp index 31499cbf9317..feb5d64190e5 100644 --- a/openmp/libomptarget/src/device.cpp +++ b/openmp/libomptarget/src/device.cpp @@ -281,7 +281,7 @@ TargetPointerResultTy DeviceTy::getTargetPointer( MESSAGE("device mapping required by 'present' map type modifier does not " "exist for host address " DPxMOD " (%" PRId64 " bytes)", DPxPTR(HstPtrBegin), Size); - } else if (PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY && + } else if (PM->getRequirements() & OMP_REQ_UNIFIED_SHARED_MEMORY && !HasCloseModifier) { // If unified shared memory is active, implicitly mapped variables that are // not privatized use host address. Any explicitly mapped variables also use @@ -445,7 +445,7 @@ DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size, bool UpdateRefCount, LR.TPR.getEntry()->dynRefCountToStr().c_str(), DynRefCountAction, LR.TPR.getEntry()->holdRefCountToStr().c_str(), HoldRefCountAction); LR.TPR.TargetPointer = (void *)TP; - } else if (PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) { + } else if (PM->getRequirements() & OMP_REQ_UNIFIED_SHARED_MEMORY) { // If the value isn't found in the mapping and unified shared memory // is on then it means we have stumbled upon a value which we need to // use directly from the host. @@ -529,7 +529,7 @@ int DeviceTy::deallocTgtPtrAndEntry(HostDataToTargetTy *Entry, int64_t Size) { void DeviceTy::init() { // Make call to init_requires if it exists for this plugin. if (RTL->init_requires) - RTL->init_requires(PM->RTLs.RequiresFlags); + RTL->init_requires(PM->getRequirements()); int32_t Ret = RTL->init_device(RTLDeviceID); if (Ret != OFFLOAD_SUCCESS) return; diff --git a/openmp/libomptarget/src/interface.cpp b/openmp/libomptarget/src/interface.cpp index 3d82b0250fae..2266dd039636 100644 --- a/openmp/libomptarget/src/interface.cpp +++ b/openmp/libomptarget/src/interface.cpp @@ -39,7 +39,7 @@ using namespace llvm::omp::target::ompt; /// adds requires flags EXTERN void __tgt_register_requires(int64_t Flags) { TIMESCOPE(); - PM->RTLs.registerRequires(Flags); + PM->addRequirements(Flags); } //////////////////////////////////////////////////////////////////////////////// diff --git a/openmp/libomptarget/src/rtl.cpp b/openmp/libomptarget/src/rtl.cpp index 52ea76438d79..afe47d6145b7 100644 --- a/openmp/libomptarget/src/rtl.cpp +++ b/openmp/libomptarget/src/rtl.cpp @@ -164,47 +164,6 @@ static __tgt_image_info getImageInfo(__tgt_device_image *Image) { return __tgt_image_info{(*BinaryOrErr)->getArch().data()}; } -void PluginAdaptorManagerTy::registerRequires(int64_t Flags) { - // TODO: add more elaborate check. - // Minimal check: only set requires flags if previous value - // is undefined. This ensures that only the first call to this - // function will set the requires flags. All subsequent calls - // will be checked for compatibility. - assert(Flags != OMP_REQ_UNDEFINED && - "illegal undefined flag for requires directive!"); - if (RequiresFlags == OMP_REQ_UNDEFINED) { - RequiresFlags = Flags; - return; - } - - // If multiple compilation units are present enforce - // consistency across all of them for require clauses: - // - reverse_offload - // - unified_address - // - unified_shared_memory - if ((RequiresFlags & OMP_REQ_REVERSE_OFFLOAD) != - (Flags & OMP_REQ_REVERSE_OFFLOAD)) { - FATAL_MESSAGE0( - 1, "'#pragma omp requires reverse_offload' not used consistently!"); - } - if ((RequiresFlags & OMP_REQ_UNIFIED_ADDRESS) != - (Flags & OMP_REQ_UNIFIED_ADDRESS)) { - FATAL_MESSAGE0( - 1, "'#pragma omp requires unified_address' not used consistently!"); - } - if ((RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) != - (Flags & OMP_REQ_UNIFIED_SHARED_MEMORY)) { - FATAL_MESSAGE0( - 1, - "'#pragma omp requires unified_shared_memory' not used consistently!"); - } - - // TODO: insert any other missing checks - - DP("New requires flags %" PRId64 " compatible with existing %" PRId64 "!\n", - Flags, RequiresFlags); -} - void PluginAdaptorManagerTy::registerLib(__tgt_bin_desc *Desc) { PM->RTLsMtx.lock(); diff --git a/openmp/libomptarget/test/offloading/requires.c b/openmp/libomptarget/test/offloading/requires.c index e5e58012a37c..cf01a73661d8 100644 --- a/openmp/libomptarget/test/offloading/requires.c +++ b/openmp/libomptarget/test/offloading/requires.c @@ -1,5 +1,7 @@ -// RUN: %libomptarget-compile-generic && env LIBOMPTARGET_DEBUG=1 %libomptarget-run-generic 2>&1 | %fcheck-generic -allow-empty -check-prefix=DEBUG -// REQUIRES: libomptarget-debug +// clang-format off +// RUN: %libomptarget-compile-generic -DREQ=1 && %libomptarget-run-generic 2>&1 | %fcheck-generic -check-prefix=GOOD +// RUN: %libomptarget-compile-generic -DREQ=2 && not %libomptarget-run-generic 2>&1 | %fcheck-generic -check-prefix=BAD +// clang-format on /* Test for the 'requires' clause check. @@ -24,11 +26,17 @@ void run_reg_requires() { // of the requires clauses. Since there are no requires clauses in this file // the flags state can only be OMP_REQ_NONE i.e. 1. - // This is the 2nd time this function is called so it should print the debug - // info belonging to the check. + // This is the 2nd time this function is called so it should print SUCCESS if + // REQ is compatible with `1` and otherwise cause an error. __tgt_register_requires(1); - __tgt_register_requires(1); - // DEBUG: New requires flags 1 compatible with existing 1! + __tgt_register_requires(REQ); + + printf("SUCCESS"); + + // clang-format off + // GOOD: SUCCESS + // BAD: omptarget fatal error 2: '#pragma omp requires reverse_offload' not used consistently! + // clang-format on } // --------------------------------------------------------------------------- |