es-ude / es-ude/OnDeviceTraining

arithmetic/executeOp: writeOut swap-restore of the target's qConfig roundingMode is not reentrant — replace with explicit rounding parameter when multi-threading lands

Open
#386 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
1
Forks
3
Avg merge
1d 1h
Merged PRs (30d)
8

Description

## Context

#282 (PR #377) made OUT_WRITE epilogue rounding operation-owned. The injection mechanism is a **transient swap** of the target's storage-rounding slot, scoped inside `writeOut`:

```c
static roundingMode_t *storageRoundingSlot(tensor_t *tensor); /* ExecuteOp.c:48-55 */
static void writeOut(tensor_t *intermediate, tensor_t *target, roundingMode_t opRounding) {
roundingMode_t *slot = storageRoundingSlot(target); /* ExecuteOp.c:69-75 */
/* swap in opRounding → run conversionMatrix encode → restore */
}
```

Chosen deliberately (documented in PR #377): the epilogue reuses the `conversionMatrix` machinery whose ~20 function signatures all read the output qConfig internally — the swap kept them untouched. Correct and tested single-threaded; the qConfig is restored before return.

## Why it can't stay forever

Project directive: MCU targets are **not** assumed single-threaded — the framework must be designed for future multi-threading. The swap-restore mutates shared state: any concurrent reader of the same tensor's qConfig (another op deriving `arithmeticFromQuantization`, serialization, a second op writing the same target) can observe the temporarily-swapped rounding mode, or two writers can lose a restore. This is the textbook non-reentrant idiom.

## Proposed direction

When the first multi-threading work is scheduled (not before — there is no functional bug today), replace the swap with an **explicit rounding parameter** on the internal conversion path: either extend the `conversionMatrix` function signature or introduce a small conversion-context struct carried through `executeConvert`. `convertTensor`'s bare-conversion role keeps passing the target-config mode, so external semantics stay identical; only the injection mechanism changes.

## Relations

#282 / PR #377 (origin, tradeoff documented there), docs/conventions/arithmetic-sym.md (mechanism documented).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Contributor guide

Open the contributing guide

Research direction

Start with storageRoundingSlot and writeOut in ExecuteOp.c, then read the conversionMatrix path through executeConvert and the documented mechanism in docs/conventions/arithmetic-sym.md. Compare the existing single-threaded behavior with the multi-threading requirements and the tradeoff recorded in #282/PR #377. Done means conversion no longer depends on transient shared qConfig mutation while convertTensor retains its existing semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.