[hipDNN] [ALMIOPEN-834] - Initial version of EngineConfig Knob RFC #3667

mousdahl-amd · 2026-01-06T21:51:04Z

Motivation

We want the ability to expose engine configuration parameters (knobs) to consumers of hipDNN. This RFC presents a path forward.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

jerehartAMD

Overall looks really good!

My big piece of feedback, especially for the frontend, I don't think a vector is the correct data structure for holding the knobs and knob info, and it should probably be a map instead.

My suggestion:
In the frontend use the id as the hash in a map for the knob info, and turn KnobSetting into KnobValue and have the user pass in a map of knobId to KnobValue

jerehartAMD · 2026-01-07T16:21:28Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+}
+
+table EnumConstraints {
+    valid_values: [string];       // Required: list of valid enum strings


This feels a bit weird to me, I think that the EnumValue and EnumConstraints should be the same type. Unless we're always guaranteed a string to enum conversion function it could be cumbersome to programmatically check if the value you're using is supported.

Yeah, let's chat about this. I think I agree, but I'm not sure what the best solve would be.

I think we can chat, but I would expect this to be the list of enum values as uint or something similar.

jerehartAMD · 2026-01-07T16:25:16Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+    else
+    {
+        // Use default for other knobs
+        customKnobSettings.push_back(info.toDefaultKnob());
+    }


What are the obstacles for having the default being automatically set? Is there a function where we could loop over the knob infos and fill in the defaults if they're not overriden?

Anything not set explicitly should be the default, and it shouldn't be required to be set.

If we change this to a map, I think we can just replace the defaults (generated by knobToDefaultKnobSetting) with custom values.

I think we should use a map, but it shouldn't be providing any values from the frontend for defaults (unless user sets it).
I think it's better to make the API just a set of explicitly overridden settings.

Doing it this way makes it good for serialization (only save what the user explicitly wanted), and also reduce versioning issues if the plugin authors change their engine configuration settings.

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

jerehartAMD · 2026-01-07T16:38:31Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+};
+
+// Knob information class - describes available knobs for an engine
+class Knob {


Not a big deal, but I think it'd make things conceptually easier to follow if this name lined up with what's in the flatbuffers schema, ie: KnobInfo

We've got a few options:

We get Knob and set KnobSetting

We get KnobInfo and set Knob

We get KnobInfo and set KnobSetting

I'll come back through and rename things when we decide.

I agree on matching with schema, but can chat on the specific name choice.

Votes cast, first one wins.

jerehartAMD · 2026-01-07T16:45:49Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+    if(info.getKnobIdStr() == "miopen.conv.tile_size")
+    {
+        // Override tile size
+        customKnobSettings.emplace_back(info.getKnobId(), 32);
+    }


Needing to iterate through these is a little clunky, it'd be nice if there were some sort of map-like interface, possibly mapping knob id to its constraints. That also has the advantage of making it so you can only ever have one knob info for a given knob id

I like the idea of being able to write:

if(knobs.contains(knobIdToStr("miopen.conv.tile_size")){ customKnobSettings.emplace_back(info.getKnobId(), 32); }

Or a more complex type than vector as well that could give you syntax like:
if(knobs.supports("miopen.conv.tile_size"))

I think the knob settings should probably be a map too if that's possible. If anything, I think that's more important since it's managed by the user

I like that. Doing it.

I also think we should be requiring these get documented by plugin authors, and having a header in the frontend for defining these for easy use.

This would allow us to still support custom plugin knobs, but make explicit choices available and easy to consume for users of hipDNN frontend.

We likely should support both setting them via name, or Id, and manage the conversion ourselves using the hashing.

jerehartAMD · 2026-01-07T16:58:12Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+1. **Multiple Value Types**: int64, float64, bool, string, and enumerated types
+2. **Flexible Validation**: Per-knob validation rules appropriate to the value type
+3. **Plugin Autonomy**: Plugins can define custom knobs without modifying core hipDNN
+4. **Namespace Isolation**: Different plugins can have knobs with the same semantic meaning without conflicts


I can see where it's important to avoid conflicts between knobs within an engine, but where is it necessary across engines? Aren't you always setting or getting from a specific engine?

I guess that's a good thing for us to chat about. I was thinking for serialization purposes it might avoid headaches, but maybe not?

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

BrianHarrisonAMD · 2026-01-08T16:55:45Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+    display_name: string;         // Human-readable name
+    description: string;          // Help text
+    value_type: KnobValue;        // Discriminator for union
+    default_value: KnobValue;     // Default setting


Not sure we need this.
I think we want to know the default in user documentation, but I would expect this API to be just a set of overrides, and we don't need to provide any that are default back (unless the user decides to set it themselves).

Maybe it's worth keeping just for logging purposes.

I feel like it has utility because the knobs are plugin vendor supplied, and we don't control how they're documented, so this is a bit of a forcing function.

BrianHarrisonAMD · 2026-01-08T17:05:00Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+    void applyKnobSettings(const KnobSettings& settings) override {
+        // Extract and apply settings
+        if (auto tileSize = settings.getInt("miopen.conv.tile_size")) {
+            _tileSize = *tileSize;
+        }
+        if (auto algo = settings.getEnum("miopen.conv.algorithm")) {
+            _algorithm = parseAlgorithm(*algo);
+        }
+        if (auto reuse = settings.getBool("miopen.conv.reuse_workspace")) {
+            _reuseWorkspace = *reuse;
+        }
+    }
+
+private:
+    int64_t _tileSize = 16;
+    Algorithm _algorithm = Algorithm::Winograd;
+    bool _reuseWorkspace = true;


I think these properties exist at the execution plan level.
They should be serialized with the execution plan, and the engine shouldn't be stateful with them I believe.

We likely need some way to register the properties that are available at the engine, but they get set and stored with the plan itself. The properties would then be used during the build stage, or execute stage of the plan.

It would be up to the plan to decide if it needs to keep the properties around.
Some are likely build time properties, and others are runtime properties.

BrianHarrisonAMD · 2026-01-08T17:15:00Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+    // Get default value (type-specific)
+    std::optional<int64_t> getDefaultInt() const;
+    std::optional<double> getDefaultFloat() const;
+    std::optional<bool> getDefaultBool() const;
+    std::optional<std::string> getDefaultString() const;


I do like that if we remove the default value then this goes away.

BrianHarrisonAMD · 2026-01-08T17:20:22Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+// Option 1: Use default knob values
+auto defaultKnobSettings = Graph::knobToDefaultKnobSettings(knobs);
+graph.create_execution_plan_ext(MIOPEN_ENGINE, defaultKnobSettings);


Similar to the above, I think no reason to have any default's.
The current API that does it without knobs should pass no knobs into the backend, and will use defaults for all properties.

I guess this also means for the documentation we should make it clear that all knob settings are optional, and a default must always be provided by the plugin authors.

CMiservaAMD · 2026-01-08T17:20:36Z

This looks really good @mousdahl-amd!

Some higher-level thoughts/questions:

A similar question was already asked about needing namespacing for the knobs, but if we are going to namespace, will that be enforced by the sdk or will we be leaving it up to the plugin developers to ensure their settings are namespaced? If it's the latter then we likely won't be able to depend on this because it will fall apart as soon as one plugin developer doesn't namespace their knobs.
Is there a way for the plugin to ultimately provide an "invalid configuration" message back to the backend for situations where, say, different knob values together are invalid?
And from a maintenance perspective, I'm wondering if we might be able to start with only the int knob (with the valid values array) and the float knob, and then expand to include more as we see how users want to use the knobs:
- An enum could be implemented as an int with a valid values array. This might be clearer for users as well as (iicu) with enums the enum knob value would be set with an int index into the enum array, which would be a level of indirection that might make things more difficult than if they configured using the enum values directly as int knobs when they set the knob.
- Since a string needs to have specific values, that could be implemented using enums / an integer with valid values array.
- A bool could be an int with valid values 0 and 1. This might be better for users long-term as often a setting that starts as a bool ends up needing to be converted to an enum in the future anyways.

Just some thoughts. The RFC looks good to me as-is too. And sorry for the last minute review.

adickin-amd · 2026-01-08T17:23:05Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+    min_value: int64;
+    max_value: int64;
+    stride: int64 = 1;            // Step size (default 1)
+    valid_values: [int64];        // Optional: explicit list of valid values


is this for the user to use to pick the value? If the user sets something invalid, is it the plugin that would reject the knob setting?

BrianHarrisonAMD · 2026-01-08T17:27:15Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+To prevent conflicts between plugins, we recommend a hierarchical naming scheme:
+
+```
+<plugin_name>.<engine_name>.<knob_name>
+
+Examples:
+- miopen.conv.tile_size
+- miopen.conv.algorithm
+- rocblas.gemm.transpose_algorithm
+- custom_plugin.matmul.block_size
+```


I think having a requirement that these are declared in the frontend also will allow us to check for overlaps.
For custom plugins we can't enforce this, but that's not a big issue.

I think it's also fine if they overlap, they don't actually have to be globally unique as far as I can tell.

BrianHarrisonAMD · 2026-01-08T17:28:53Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+
+**Rationale**:
+- **Plugin Autonomy**: Plugins can define custom knobs without modifying core hipDNN
+- **Collision Avoidance**: Namespace prefixes prevent conflicts


I think we should strip this concern out of the RFC.
I don't see a reason why an overlapping knob would be problematic, it just needs to be unique within the engine itself.

BrianHarrisonAMD · 2026-01-08T17:30:04Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+### 5.4 Validation at Finalization
+
+**Decision**: Validate knob settings during `hipdnnBackendFinalize(engineCfg)`.
+
+**Rationale**:
+- **Early Error Detection**: Catch invalid settings before execution
+- **Clear Error Messages**: Can provide detailed validation errors
+- **Consistency**: Matches existing finalization pattern in hipDNN


I think it would be nice to have sanity checks in the frontend from the provided definition if we can manage it.
Since it's provided from the backend, I think we could do it at that level, and also the plugin level itself as a last resort (this will catch issues with breaking engine config changes that were serialized).

BrianHarrisonAMD · 2026-01-08T17:31:22Z

projects/hipdnn/docs/rfcs/0004_EngineConfigKnobs.md

+### 5.5 Serialization Support
+
+**Decision**: Engine configurations with knob settings are serializable via Flatbuffers.
+
+**Rationale**:
+- **Persistence**: Users can save optimal configurations
+- **Reproducibility**: Configurations can be shared across runs/systems
+- **Debugging**: Serialized configs can be inspected and modified
+- **Integration**: Works naturally with existing Flatbuffer infrastructure


I guess this is implied, but we might want to call out that these will not be serialized with the graph, but instead serialized with the execution plans.

Serializing execution plans isn't existing functionality today, but will exist in the future.

Initial version of EngineConfig Knob RFC

78e562a

mousdahl-amd requested a review from a team as a code owner January 6, 2026 21:51

github-actions bot added documentation ci:docs-only Docs only changes project: hipdnn labels Jan 6, 2026

assistant-librarian bot added the organization: ROCm label Jan 6, 2026

jerehartAMD reviewed Jan 7, 2026

View reviewed changes

Updated from PR review

a23b57c

BrianHarrisonAMD reviewed Jan 8, 2026

View reviewed changes

adickin-amd reviewed Jan 8, 2026

View reviewed changes

BrianHarrisonAMD reviewed Jan 8, 2026

View reviewed changes

PR and meeting comment updates

8d22295

[hipDNN] [ALMIOPEN-834] - Initial version of EngineConfig Knob RFC #3667

Are you sure you want to change the base?

[hipDNN] [ALMIOPEN-834] - Initial version of EngineConfig Knob RFC #3667

Conversation

mousdahl-amd commented Jan 6, 2026

Motivation

Submission Checklist

Uh oh!

jerehartAMD left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mousdahl-amd Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrianHarrisonAMD Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CMiservaAMD commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrianHarrisonAMD Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerehartAMD left a comment •

edited

Loading

mousdahl-amd Jan 8, 2026 •

edited

Loading

BrianHarrisonAMD Jan 8, 2026 •

edited

Loading

CMiservaAMD commented Jan 8, 2026 •

edited

Loading

BrianHarrisonAMD Jan 8, 2026 •

edited

Loading