Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Generalize default transformation pipeline

Open PathogenDavid opened this issue 3 years ago • 1 comments

Edit in 2022: I've been doing a lot of work on this lately. I prototyped some things and I've settled on something similar to the mutable pipelines stuff below.

Right now the pipelines API lives separately from Biohazrd in a private repo which uses Biohazrd. Things that need to be done before we upstream it:

  • [ ] Clean up internal jank around stopwatches which could be better handled by using the debugger API.
  • [ ] Decide if/how to handle pipeline splitting for advanced generators like Mochi.DirectX
  • [ ] Semi-related: Remove old function emit logic as mentioned in https://github.com/MochiLibraries/Biohazrd/issues/236
  • [ ] Add some mechanism for emit options to be available to trampolines.

(This is a bit of a ramble just to make a record of needing to do this along with my ideas.)

Right now Biohazrd expects each generator author to write out a common pipeline of transformations. Biohazrd is not especially useful without many elements of this pipeline, but at the same time there's valuable in them being optional for advanced users. (In particular, being able to drop or customize built-in transformations is very useful when handling a C++ feature Biohazrd doesn't support yet.) It also makes sense that a generator author might not necessarily want to introduce changes to their generated library by upgrading Biohazrd, but instead would like to be notified what those changes are.

One major issue to solve here is making sure generators can evolve with Biohazrd as appropriate. For example, 120b8de8aa6952950d21866c9fa79df74ac0df5f introduced WrapNonBlittableTypesWhereNecessaryTransformation as a workaround for https://github.com/InfectedLibraries/Biohazrd/issues/99. Not having this transformation (or something similar) can lead to very annoying bugs in the output (although it should result in a verification warning/error -- created https://github.com/InfectedLibraries/Biohazrd/issues/101)

Additionally it would be nice if pipelines were defined rather than immediately executed. This would make it easier to create a pipeline debugger tool eventually.

I've been going back and forth on how I want to do this.

Mutable default pipelines

One idea is to have a default pipeline builder which you modify by inserting your own transformations. IE, something like this:

// 0 is the version of the pipeline, breaking changes would create new pipeline versions
TransformationPipeline pipeline = new DefaultCSharpPipeline0();
pipline.InsertBefore<RemoveExplicitBitFieldPaddingFieldsTransformation>
(
    new RemoveBadPhysXDeclarationsTransformation(),
    new PhysXRemovePaddingFieldsTransformation(),
    new PhysXEnumTransformation(),
    new PhysXFlagsEnumTransformation()
);

One issue with this strategy is it becomes somewhat brittle in that you're using knowledge of the default pipeline to insert your own stages. This is part of why I am defining the concept of pipeline versions where you need to bump the version number to get the new pipeline.

One possible workaround for this problem is to instead define a set of insertion points where generator authors would typically want to add their customizations. (For example: BeforeAnything, BeforeConvienenceTransforms, AfterTypeReduction, etc. Defining these points might be annoying though. There is some semblance of stages in the current "default" pipeline, but they aren't well-defined.

The other issue with this is that it obfuscates the pipeline. This could be alleviated by a pipeline debugger tool.

Create combined pipeline stages

A variant of the insertion point plans is to provide a way to combine transformations into groups and allow referencing them instead. This would remove the amount of perceived boilerplate from generators and give Biohazrd a way to evolve the default pipeline when necessary.

Using an analyzer

Another possibility is that we could instead ship an analyzer+codefix with Biohazrd that can populate a default pipeline and let you know when new stages have been added to the default pipeline and where you should add them.

For example, when we recently introduced the new WrapNonBlittableTypesWhereNecessaryTransformation, the analyzer could inform the generator author of the new transformation stage and where to add it. (Along with a code fix that would just insert it for them.)

This might be my favorite idea right now because it makes modifying the transformation pipeline more explicit. (Obviously existing transformations might change, but ideally their changes are more subtle.)

PathogenDavid avatar Nov 28 '20 18:11 PathogenDavid

Minor tweaks that should be done to the default order used in infected libraries today:

  • CSharpBuiltinTypeTransformation should be right after CSharpTypeReductionTransformation.
  • DeduplicateNamesTransformation should be last before CSharpTranslationVerifier.
  • [ ] InfectedImGui
  • [ ] InfectedPhysX
  • [ ] InfectedTensorRT

PathogenDavid avatar Nov 30 '20 19:11 PathogenDavid