Split atomic exchange into load + store

morrisonlevi · morrisonlevi · commit 8fa433a16aac · 2024-08-14T11:41:14.000-06:00
It is difficult to determine performance of atomics sometimes. In this
case, the separate load+store is still correct, and a load does not
cause a modification, and might be faster for some platforms than an
exchange. A load+store is slower than an exchange, but we're fine
trading the penalty to the slow path and keeping the happy path faster.
diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -4054,8 +4054,9 @@ ZEND_API void ZEND_FASTCALL zend_free_compiled_variables(zend_execute_data *exec
 }
 /* }}} */
 
-ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call)
+ZEND_API ZEND_COLD void ZEND_FASTCALL zend_fcall_interrupt(zend_execute_data *call)
 {
+	zend_atomic_bool_store_ex(&EG(vm_interrupt), false);
 	if (zend_atomic_bool_load_ex(&EG(timed_out))) {
 		zend_timeout();
 	} else if (zend_interrupt_function) {
@@ -4076,7 +4077,7 @@ ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call)
 	} while (0)
 
 #define ZEND_VM_FCALL_INTERRUPT_CHECK(call) do { \
-		if (UNEXPECTED(zend_atomic_bool_exchange_ex(&EG(vm_interrupt), false))) { \
+		if (UNEXPECTED(zend_atomic_bool_load_ex(&EG(vm_interrupt)))) { \
 			zend_fcall_interrupt(call); \
 		} \
 	} while (0)
diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h
@@ -539,10 +539,10 @@ ZEND_COLD void zend_magic_get_property_type_inconsistency_error(const zend_prope
 
 ZEND_COLD void zend_match_unhandled_error(const zval *value);
 
-/* Call this to handle the timeout or the interrupt function. It will not clear
- * the EG(vm_interrupt); this is done by the fcall/jit handler.
+/* Call this to handle the timeout or the interrupt function. It will set
+ * EG(vm_interrupt) to false.
  */
-ZEND_API ZEND_COLD void zend_fcall_interrupt(zend_execute_data *call);
+ZEND_API ZEND_COLD void ZEND_FASTCALL zend_fcall_interrupt(zend_execute_data *call);
 
 static zend_always_inline void *zend_get_bad_ptr(void)
 {
diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c
@@ -3272,9 +3272,3 @@ static zend_string* ZEND_FASTCALL zend_jit_rope_end(zend_string **rope, uint32_t
 	*target = '\0';
 	return ret;
 }
-
-ZEND_COLD static void ZEND_FASTCALL zend_jit_fcall_interrupt(zend_execute_data *call)
-{
-	zend_atomic_bool_store_ex(&EG(vm_interrupt), false);
-	zend_fcall_interrupt(call);
-}
diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c
@@ -3075,7 +3075,7 @@ static void zend_jit_setup_disasm(void)
 	REGISTER_HELPER(zend_jit_pre_dec_obj_helper);
 	REGISTER_HELPER(zend_jit_post_dec_obj_helper);
 	REGISTER_HELPER(zend_jit_rope_end);
-	REGISTER_HELPER(zend_jit_fcall_interrupt);
+	REGISTER_HELPER(zend_fcall_interrupt);
 
 #ifndef ZTS
 	REGISTER_DATA(EG(current_execute_data));
@@ -10203,10 +10203,10 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen
 		 * down that handles when there isn't an interrupt function.
 		 */
 		if (zend_interrupt_function) {
-			// JIT: if (EG(vm_interrupt)) zend_jit_fcall_interrupt(execute_data);
+			// JIT: if (EG(vm_interrupt)) zend_fcall_interrupt(execute_data);
 			ir_ref if_interrupt = ir_IF(ir_LOAD_U8(jit_EG(vm_interrupt)));
 			ir_IF_TRUE_cold(if_interrupt);
-			ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_fcall_interrupt), rx);
+			ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_fcall_interrupt), rx);
 			ir_MERGE_WITH_EMPTY_FALSE(if_interrupt);
 		}
 
