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
- 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
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