Skip to content
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

Thunks: Add new pointer annotations to assist data layout analysis #3177

Merged

Conversation

neobrain
Copy link
Member

@neobrain neobrain commented Oct 4, 2023

Overview

This PR adds a number of annotations related to pointer struct members and pointer function parameters. This allows the data layout analysis framework added in #3156 to operate on a wider range of thunked APIs.

The initial version of the data layout analysis framework is limited to "simple" structures. "Simple" means that pointer members are only supported if they pointee-type was inferred to have compatible data layout itself. This excludes a couple of important use cases:

  • Structs containing pointers to "opaque" types that have no visible definition (where the referenced data is only ever created/read/written on the host side)
  • Structs containing types that have consistent data layout across architectures, but the data layout analysis module doesn't support processing them yet (e.g. union types)
  • Functions where only one or two parameters are pointers to non-compatible data (i.e. automation is desired for the other parameters but not supported yet)
  • Self-referencing structs such as linked-list

To handle these use cases, two new types of annotations are added (in contrast to fex_gen_config, which describes only functions):

  • Parameter annotations: fex_gen_param describes specific parameters within a function
  • Type annotations: fex_gen_type describes types, such that the described behavior will be applied to all functions/structs they are used in

Implementation

opaque_type/assume_compatible_data_layout annotations

These instruct data analysis to assume the underlying data is compatible across all architectures. assume_compatible_data_layout can be used for both pointers and non-pointers, whereas opaque_type ensures the type is only ever used as a pointer.

Importantly, this means assume_compatible_data_layout can handle types embedded as non-pointers in structs. This should only be used over opaque_type if the layout has actually been manually inspected though, of course.

opaque_type is purely a type annotation (since this property shouldn't depend on context), whereas assume_compatible_data_layout may be used as either a type annotation or a function parameter annotation.

Typical use cases are "handle types", union types, and linked list types.

ptr_passthrough annotation

This function parameter annotation is used to force passing individual function parameters without modification to a custom host implementation (fexgen::custom_host_impl) even if the parameter type is known to differ between architectures. A new guest_layout wrapper template is added to make this behavior explicit: The argument for the host function then changes in type from T* to guest_layout<T*>.

For example:

 int fexfn_impl_wl_proxy_add_listener
        wl_proxy *proxy,
        guest_layout<void (**)()> array_of_callbacks,
        guest_layout<void*> data) { ... }

For now, guest_layout is a loaded foot gun: It provides no extra guarantees or layout conversion to deal with data layout incompatibilities. A later PR will remedy this. Even now the wrapper adds value though, because we've been using the foot gun all along but we didn't make it explicit.

Typical use cases are pointers-to-pointers, function-pointer-parameters. More generally, it allows for complex manual repacking of struct arguments.

Embed annotations into the signature for Vulkan-esque function pointers

Some APIs (prominently vkGetDeviceProcAddr or glXGetProcAddr) return host-function pointers to the guest. The trampolines we insert guest-side to handle this used to be unique per-signature, but additionally they now need to be unique per set-of-annotations.

Since this adds up to a lot of information to carry around at compile time, this turns template<typename Result, typename... Args> void ForIndirectCall(void*); into this rather heavy code:

template<typename Result, typename... Args>
struct GuestWrapperForHostFunction {
  template<ParameterAnnotations... Annotations>
  void Call(void* argsv);
};

Impact

Since strict mode (see #3156) is still only enabled for unit tests, impact on real libraries is limited. Functionally there should be no difference, but the generated code will look slightly different for functions using ptr_passthrough. Similarly, since Vulkan-esque function pointers now carry around annotation at compile-time, the callback list is slightly different.

C++20 is required for the callback change due to the use of non-type template parameters. This version bump could be delayed but other features in my PR queue will also require C++20. If C++17 support is desirable, I propose temporarily requiring C++20 and restoring C++17 support once this PR series is complete.

Future

The next PR after this will add the new annotations to all of our existing thunk libraries. We'll then be able to turn on strict mode for data layout analysis unconditionally.

ThunkLibs/Generator/analysis.cpp Outdated Show resolved Hide resolved
@@ -184,9 +221,14 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) {
type = type->getLocallyUnqualifiedSingleStepDesugaredType();

if (type->isFunctionPointerType() || type->isFunctionType()) {
funcptr_types.insert(type.getTypePtr());
funcptr_types["TODO_FUNC_NAME_FOR_ANNOTATED_TYPES_" + type.getAsString()] = std::pair { type.getTypePtr(), no_param_annotations };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisit this. Adding a fixed prefix hasn't caused any problems so far, but to be on the safe side it may indeed be needed to add the full function name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is safe, since the type name for a function pointer type will never conflict with a function name.

ThunkLibs/Generator/analysis.cpp Outdated Show resolved Hide resolved
ThunkLibs/Generator/analysis.h Outdated Show resolved Hide resolved
ThunkLibs/Generator/data_layout.cpp Outdated Show resolved Hide resolved
@Sonicadvance1
Copy link
Member

C++20 is required for the callback change due to the use of non-type template parameters. This version bump could be delayed but other features in my PR queue will also require C++20. If C++17 support is desirable, I propose temporarily requiring C++20 and restoring C++17 support once this PR series is complete.

We already compile both FEXCore and FEX's frontend as C++20, no reason for thunks to require anything older.

Copy link
Member

@Sonicadvance1 Sonicadvance1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me with a first pass.

@neobrain neobrain force-pushed the feature_thunk_pointer_annotations branch 3 times, most recently from 7660033 to 8b5d111 Compare October 18, 2023 15:19
This reflects its purpose slightly better, particularly since future patches
will add more information to this object.
Previously, two functions with the same signature would always be wrapped
in the same logic. This change allows customizing one function with
annotations while leaving the other one unchanged.
…back arguments

This will be used later to aid automatic struct repacking.
This annotation can be used for data types that can't be repacked
automatically even with custom repack annotations. With ptr_passthrough,
the types are wrapped in guest_layout and passed to the host like that.
These annotations allow for a given type or parameter to be treated as
"compatible" even if data layout analysis can't infer this automatically.

assume_compatible_data_layout is more powerful than is_opaque, since it
allows for structs containing members of a certain type to be automatically
inferred as "compatible".

Conversely however, is_opaque enforces that the underlying data is never
accessed directly, since non-pointer uses of the type would still be
detected as "incompatible".
@neobrain neobrain force-pushed the feature_thunk_pointer_annotations branch from 8b5d111 to cb215b5 Compare October 19, 2023 10:49
@Sonicadvance1 Sonicadvance1 merged commit 978f607 into FEX-Emu:main Oct 23, 2023
10 checks passed
@neobrain neobrain deleted the feature_thunk_pointer_annotations branch October 23, 2023 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants