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

Remove usage of nested PassManager from the compiler #1036

Closed
5 tasks
benvanik opened this issue Mar 11, 2020 · 3 comments · Fixed by #3741
Closed
5 tasks

Remove usage of nested PassManager from the compiler #1036

benvanik opened this issue Mar 11, 2020 · 3 comments · Fixed by #3741
Assignees
Labels
compiler/dialects Relating to the IREE compiler dialects (flow, hal, vm)

Comments

@benvanik
Copy link
Collaborator

benvanik commented Mar 11, 2020

To make debugging easier I think it'd be good to remove the nested pass manager used during executable translation... somehow.

Right now, all executables for all targets are translated in a single parent pass (https://github.com/google/iree/blob/master/iree/compiler/Dialect/HAL/Transforms/TranslateExecutables.cpp#L76) which then requires the various targets to initiate their own pass run (https://github.com/google/iree/blob/master/iree/compiler/Dialect/HAL/Target/VMLA/VMLATarget.cpp#L70-L75).

If we instead move the logic for deriving target backends to the transform pipeline setup (https://github.com/google/iree/blob/master/iree/compiler/Dialect/HAL/Transforms/Passes.cpp#L33) we could have the target registration function populate a nested pass manager (nest<IREE::HAL::ExecutableOp>()) with the passes it requires.

This should make getting end-to-end reproducers easier.

Right now the target functions may do a bit of extra prep that we should remove or move to passes (https://github.com/google/iree/blob/master/iree/compiler/Dialect/HAL/Target/VMLA/VMLATarget.cpp#L59-L67) before running their core passes (https://github.com/google/iree/blob/master/iree/compiler/Dialect/HAL/Target/VMLA/VMLATarget.cpp#L71-L72) and finally serializing their contents (https://github.com/google/iree/blob/master/iree/compiler/Dialect/HAL/Target/VMLA/VMLATarget.cpp#L77-L111).

If instead each target had a serialization pass that took the contents and packed them into the flatbuffer then we don't need to run anything outside of the nested pass.

Initial tasks (unblocked):

  • Change ExecutableTargetRegistration (maybe to an interface with a populatePasses method to make future work around target specialization easier)
  • Move LLVM/VMLA/SPIR-V serialization to their own passes in their respective dialects
  • File a bug with upstream because mlir::translateModuleToLLVMIR does the same pass manager nesting - it shouldn't (only impacts llvmjit backend)
  • Move RewriteLegacyIO pass into the target pass pipelines (not via makeLegacyExecutableABI)

Post-handwritten-VulkanSPIRVTarget.cpp-kernels:

  • Move backend list logic to HAL/Transforms/Passes.cpp and build nested pass manager
    (the logic in VulkanSPIRVTarget.cpp is complex right now with all the special cases/flags, but will be getting simpler soon)
@benvanik
Copy link
Collaborator Author

Mahesh points out that currently the SPIR-V/LLVM backends are using the RewriteLegacyIO pass - that'll also need to be setup in their new pipelines nested on the IREE::HAL::ExecutableSourceOp
(or, depending on when this work is done, may not be needed at all)

@benvanik benvanik added the compiler/dialects Relating to the IREE compiler dialects (flow, hal, vm) label Mar 19, 2020
@benvanik benvanik added this to the 2020Q2 Core milestone Mar 26, 2020
@benvanik
Copy link
Collaborator Author

benvanik commented Apr 8, 2020

Chatting with river/mehdi it sounds like dynamic pass registration will be required to do this right. They've got plans to implement that this quarter, so my hope is to get everything else done except for the unnesting and then that can be done as a simple change when the infra is ready.

@benvanik
Copy link
Collaborator Author

Filed upstream issue here: https://bugs.llvm.org/show_bug.cgi?id=45643

@benvanik benvanik modified the milestones: 2020Q2 Core, 2020Q3 Core May 5, 2020
GMNGeoffrey added a commit that referenced this issue Sep 2, 2020
Mostly solves #2958, but there are some TODOs left because this doesn't
handle our pipeline-within-a-pipeline workaround for the absence of
dynamic pass registration very gracefully. Hopefully #1036 will solve
some of these issues.
@benvanik benvanik modified the milestones: 2020Q3 Core, 2020Q4 Core Sep 12, 2020
GMNGeoffrey added a commit to GMNGeoffrey/iree that referenced this issue Nov 6, 2020
This avoids needing to register all dialects that a TargetBackend
transformation will create because we can now create the pipeline early
and query that for its dependent dialects.

Now TargetBackends are resposible only for declaring the dialects they
create entities from in declareTargetOps, which is a localized thing.

Resolves iree-org#1036
GMNGeoffrey added a commit that referenced this issue Nov 6, 2020
This avoids needing to register all dialects that a TargetBackend
transformation will create because we can now create the pipeline early
and query that for its dependent dialects.

Now TargetBackends are resposible only for declaring the dialects they
create entities from in declareTargetOps, which is a localized thing.

It also nicely propagates pass manager options and gives us parallelism
at the ExecutableTarget level.

Resolves #1036
@GMNGeoffrey GMNGeoffrey self-assigned this Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/dialects Relating to the IREE compiler dialects (flow, hal, vm)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants