[1/N] Add BackendOptions class

[cccclai]

Stack from ghstack (oldest at bottom):

• [7/N] Add sugar syntax for module.update #11534
• [6/N] Add update in module #11533
• [5/N] Add update in method #11463
• [4/N] Add backend options map #11462
• [3/N] Add update function in backend interface #11391
• [2/N] Add update context #11390
• -> [1/N] Add BackendOptions class #11389

Introduce backend option as discussed in #10216

Step 1: Introducd Backend Option class

In later stage, it will be plugged in with the rest of the stack.

Differential Revision: D75993712

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11389

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit ee878bd with merge base ef1d2ff ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D75993712

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[cccclai]

Previous comment in the old PR: #11288

From @JacobSzwejbka :

use int64_t for int type.

[cccclai]

From @mergennachin:

I don't think we need to define a generic BackendOption that takes arbitrary key value. Each backend owners could define their options instead. Vulkan will define its own config structure, Core ML will define its own config structure. Wdyt?

@JacobSzwejbka, @mcr229, @kimishpatel

Something like this:

BackendInterface { // parent interface
  virtual Error setOption(const BackendOptions& options) = 0;
}

struct BackendOptions {  // Empty parent struct
  virtual ~BackendOptions() = default;
};

Each individual backends define their own configs

struct VulkanBackendOptions : public BackendOptions {
  const char* metadata[20] = "";
  int thread_count = 1;
  // ... other Vulkan-specific options
};

VulkanBackend extends BackendInterface {
  Error setOption(const BackendOptions& options) override {
    auto* vulkan_opts = dynamic_cast<const VulkanBackendOptions*>(&options);
  }
}

// User code

VulkanBackendOptions opts;
opts.thread_count = 5;
auto backend = std::make_unique<VulkanBackend>();
backend->setOption(opts);  // 

[cccclai]

My response:

I thought about this idea at the beginning, but it means that users code needs to link to a specific backend, otherwise the runner code will fail to build. Currently the same user code can be backend agnostic, meaning the same code can still run with different backends with/without linking the backend. As the example, the portable runner can work with/without xnnpack https://github.com/pytorch/executorch/blob/main/examples/xnnpack/executor_runner/targets.bzl.

It also exposes the backend specific symbol to users, and the contract interface is a bit looser. Previously, The contract is like backend <-> ET <-> backend, and if we change it to this, it can be user <-> backend. And users can pass arbitrary things to backend. Users can run backend->init, backend->execute in their own code too, and it will be out of our control.

With the proposed solution (#10216), the user code can still be backend agnostic, and the backend option is still optional. We are still on the hook of the contract.

[kimishpatel]

I think what @mergennachin suggested makes sense. If user code is setting backend specific option they already have to know what backend they are talking about, unless some options are backend agnostic.

Other piece of the puzzle is how the user will update this. And from #10216 it seems that we will pipe update (I would call set_backend_options) method that will set all the options. But part that is not clear to me is how would user indicate which backend they setting this option for. So somehow we need a string identifer for the backends that can be used to figure out which backend's update method be called. @mergennachin in this case we cannot quite have what you proposed. Imagine you have a pointer to an instance of VulkanBackendOptions. You pass this to runtime's update method which accepts BackendOptions*. Eventually you want to call VulkanBackend's update options method so you need to cast BackendOptions* to VulkanBackendOptions*. You have two choices either do static_cast or dynamic_cast. The later requires rtti which we dont allow. For static_cast, it has to be safe and you need to know that BackendOption* is actually pointing to VulkanBackendOption. To do that you need to have BackendOptions store tag containing what is the type of the derived class. If we do that, then what you suggest can work.

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D75993712

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D75993712