Revert to prefer MTLEvent over MTLFence for VkSemaphore, except on NV…

…IDIA. Prefer MTLEvent over MTLFence for VkSemaphore, because MTLEvent handles sync across MTLCommandBuffers and MTLCommandQueues, except on NVIDIA GPUs, which have demonstrated trouble with MTLEvents, prefer MTLFence. Add MVKDevice::VkSemaphoreStyle enum.
KhronosGroup · Aug 10, 2021 · 8e6731f · 8e6731f
1 parent f969644
commit 8e6731f
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 25 deletions.
diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md
@@ -18,6 +18,7 @@ MoltenVK 1.1.5
 
 Released TBD
 
+- Revert to prefer `MTLEvent` over `MTLFence` for `VkSemaphore`, except on NVIDIA.
 - Vulkan timestamp query pools use Metal GPU counters when available.
 - Support resolving attachments with formats that Metal does not natively resolve.
 - Fix issue where swapchain images were acquired out of order under heavy load.

diff --git a/MoltenVK/MoltenVK/API/vk_mvk_moltenvk.h b/MoltenVK/MoltenVK/API/vk_mvk_moltenvk.h
@@ -557,14 +557,15 @@ typedef struct {
 	/**
 	 * Use MTLFence, if it is available on the device, for VkSemaphore synchronization behaviour.
 	 *
-	 * This parameter interacts with semaphoreUseMTLEvent. If both are enabled, semaphoreUseMTLFence
-	 * takes priority and MTLFence will be used if it is available, otherwise MTLEvent will be used
-	 * if it is available. If neither semaphoreUseMTLFence or semaphoreUseMTLEvent are enabled, or
-	 * if neither MTLFence or MTLEvent are available, CPU-based synchoronization will be used.
+	 * This parameter interacts with semaphoreUseMTLEvent. If both are enabled, on GPUs other than
+	 * NVIDIA, semaphoreUseMTLEvent takes priority and MTLEvent will be used if it is available,
+	 * otherwise MTLFence will be used if it is available. On NVIDIA GPUs, the opposite priority
+	 * applies. If neither semaphoreUseMTLFence nor semaphoreUseMTLEvent are enabled, or if neither
+	 * MTLEvent nor MTLFence are available, CPU-based synchoronization will be used.
 	 *
 	 * In the special case of VK_SEMAPHORE_TYPE_TIMELINE semaphores, MoltenVK will always
 	 * use MTLSharedEvent if it is available on the platform, regardless of the values of
-	 * MVK_ALLOW_METAL_FENCES or MVK_ALLOW_METAL_EVENTS.
+	 * semaphoreUseMTLEvent or semaphoreUseMTLFence.
 	 *
 	 * The value of this parameter must be changed before creating a VkDevice,
 	 * for the change to take effect.
@@ -573,21 +574,22 @@ typedef struct {
 	 * MVK_ALLOW_METAL_FENCES
 	 * runtime environment variable or MoltenVK compile-time build setting.
 	 * If neither is set, this setting is enabled by default, and VkSemaphore will use MTLFence,
-	 * if it is available.
+	 * if it is available, unless MTLEvent is available and semaphoreUseMTLEvent is enabled.
 	 */
 	VkBool32 semaphoreUseMTLFence;
 
 	/**
 	 * Use MTLEvent, if it is available on the device, for VkSemaphore synchronization behaviour.
 	 *
-	 * This parameter interacts with semaphoreUseMTLFence. If both are enabled, semaphoreUseMTLFence
-	 * takes priority and MTLFence will be used if it is available, otherwise MTLEvent will be used
-	 * if it is available. If neither semaphoreUseMTLFence or semaphoreUseMTLEvent are enabled, or
-	 * if neither MTLFence or MTLEvent are available, CPU-based synchoronization will be used.
+	 * This parameter interacts with semaphoreUseMTLFence. If both are enabled, on GPUs other than
+	 * NVIDIA, semaphoreUseMTLEvent takes priority and MTLEvent will be used if it is available,
+	 * otherwise MTLFence will be used if it is available. On NVIDIA GPUs, the opposite priority
+	 * applies. If neither semaphoreUseMTLFence nor semaphoreUseMTLEvent are enabled, or if neither
+	 * MTLEvent nor MTLFence are available, CPU-based synchoronization will be used.
 	 *
 	 * In the special case of VK_SEMAPHORE_TYPE_TIMELINE semaphores, MoltenVK will always
 	 * use MTLSharedEvent if it is available on the platform, regardless of the values of
-	 * MVK_ALLOW_METAL_FENCES or MVK_ALLOW_METAL_EVENTS.
+	 * semaphoreUseMTLEvent or semaphoreUseMTLFence.
 	 *
 	 * The value of this parameter must be changed before creating a VkDevice,
 	 * for the change to take effect.
@@ -596,7 +598,7 @@ typedef struct {
 	 * MVK_ALLOW_METAL_EVENTS
 	 * runtime environment variable or MoltenVK compile-time build setting.
 	 * If neither is set, this setting is enabled by default, and VkSemaphore will use MTLEvent,
-	 * if it is available, unless if MTLFence is available and semaphoreUseMTLFence is enabled.
+	 * if it is available.
 	 */
 	VkBool32 semaphoreUseMTLEvent;
 

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h b/MoltenVK/MoltenVK/GPUObjects/MVKDevice.h
@@ -832,11 +832,17 @@ class MVKDevice : public MVKDispatchableVulkanAPIObject {
 	id<MTLBuffer> _dummyBlitMTLBuffer;
     uint32_t _globalVisibilityQueryCount;
     std::mutex _vizLock;
-	bool _useMTLFenceForSemaphores;
-	bool _useMTLEventForSemaphores;
 	bool _logActivityPerformanceInline;
 	bool _isPerformanceTracking;
 	bool _isCurrentlyAutoGPUCapturing;
+
+	typedef enum {
+		VkSemaphoreStyleUseMTLEvent,
+		VkSemaphoreStyleUseMTLFence,
+		VkSemaphoreStyleUseEmulation
+	} VkSemaphoreStyle;
+	VkSemaphoreStyle _vkSemaphoreStyle;
+
 };
 
 

diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm
@@ -3263,12 +3263,10 @@ static uint32_t mvkGetEntryProperty(io_registry_entry_t entry, CFStringRef prope
 			return new MVKTimelineSemaphoreEmulated(this, pCreateInfo, pTypeCreateInfo);
 		}
 	} else {
-		if (_useMTLFenceForSemaphores) {
-			return new MVKSemaphoreMTLFence(this, pCreateInfo);
-		} else if (_useMTLEventForSemaphores) {
-			return new MVKSemaphoreMTLEvent(this, pCreateInfo);
-		} else {
-			return new MVKSemaphoreEmulated(this, pCreateInfo);
+		switch (_vkSemaphoreStyle) {
+			case VkSemaphoreStyleUseMTLEvent:  return new MVKSemaphoreMTLEvent(this, pCreateInfo);
+			case VkSemaphoreStyleUseMTLFence:  return new MVKSemaphoreMTLFence(this, pCreateInfo);
+			case VkSemaphoreStyleUseEmulation: return new MVKSemaphoreEmulated(this, pCreateInfo);
 		}
 	}
 }
@@ -3953,11 +3951,33 @@ static uint32_t mvkGetEntryProperty(io_registry_entry_t entry, CFStringRef prope
 	_pProperties = &_physicalDevice->_properties;
 	_pMemoryProperties = &_physicalDevice->_memoryProperties;
 
-	// Indicate whether semaphores should use a MTLFence or MTLEvent if they are available.
-	_useMTLFenceForSemaphores = _pMetalFeatures->fences && mvkConfig().semaphoreUseMTLFence;
-	_useMTLEventForSemaphores = _pMetalFeatures->events && mvkConfig().semaphoreUseMTLEvent;
-
-	MVKLogInfo("Using %s for Vulkan semaphores.", _useMTLFenceForSemaphores ? "MTLFence" : (_useMTLEventForSemaphores ? "MTLEvent" : "emulation"));
+	// Decide whether semaphores should use a MTLFence or MTLEvent if they are available.
+	// Prefer MTLEvent over MTLFence, because MTLEvent handles sync across MTLCommandBuffers and MTLCommandQueues,
+	// except on NVIDIA GPUs, which have demonstrated trouble with MTLEvents, prefer MTLFence.
+	bool canUseMTLEventForSem4 = _pMetalFeatures->events && mvkConfig().semaphoreUseMTLEvent;
+	bool canUseMTLFenceForSem4 = _pMetalFeatures->fences && mvkConfig().semaphoreUseMTLFence;
+	switch (_pProperties->vendorID) {
+		case kNVVendorId:
+			_vkSemaphoreStyle = canUseMTLFenceForSem4 ? VkSemaphoreStyleUseMTLFence : (canUseMTLEventForSem4 ? VkSemaphoreStyleUseMTLEvent : VkSemaphoreStyleUseEmulation);
+			break;
+		case kAppleVendorId:
+		case kIntelVendorId:
+		case kAMDVendorId:
+		default:
+			_vkSemaphoreStyle = canUseMTLEventForSem4 ? VkSemaphoreStyleUseMTLEvent : (canUseMTLFenceForSem4 ? VkSemaphoreStyleUseMTLFence : VkSemaphoreStyleUseEmulation);
+			break;
+	}
+	switch (_vkSemaphoreStyle) {
+		case VkSemaphoreStyleUseMTLEvent:
+			MVKLogInfo("Using MTLEvent for Vulkan semaphores.");
+			break;
+		case VkSemaphoreStyleUseMTLFence:
+			MVKLogInfo("Using MTLFence for Vulkan semaphores.");
+			break;
+		case VkSemaphoreStyleUseEmulation:
+			MVKLogInfo("Using emulation for Vulkan semaphores.");
+			break;
+	}
 }
 
 void MVKDevice::enableFeatures(const VkDeviceCreateInfo* pCreateInfo) {