GAP9: -O3 hot kernels; make tile-control-table memory level configurable - #199
Merged
Merged
Conversation
Compile the conv / depthwise-conv / Gemm translation units at -O3, appended last so it wins over the SDK's default -Os on the same files. Everything else stays at -Os. -O3 turns on the RISC-V (XpulpV2) hardware loops on the kernels' tight inner loops; on a forward conv the -O3 object has 18 lp.setup HW-loop instructions vs 0 at -Os, at the cost of ~+50% .text on those files.
The hoisted tile-control tables (numTiles / DMA cmd / size / dims / padding / offsets) are read-only lookup tables the cluster controller uses to drive the tiling loop and program DMAs -- not bulk tile data. Previously the L2->L1 tiling pass emitted them with _memoryLevel=self.memory="L1", so they landed in the GAP9 L1 TCDM next to the cluster master stack. On memory-tight nets this both wastes scarce L1 (~11.6 KB on CCT, ~7.0 KB on MobileNetV1, ~2.5 KB on ResNet8) and creates a correctness hazard: a deep master-stack write can clobber a single table entry, turning a DMA cmd into a garbage code pointer so mchan_transfer_wait() hangs forever (observed on MobileNetV1 training). Redirect only the L2->L1 pass to emit these tables in L2. The L3->L2 pass keeps its tables in L2 (== self.memory, unchanged). Platforms that don't tile into a level named "L1" are unaffected. Tile *data* buffers still go to L1 as before -- only the constant control tables move.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesTiling control-table memory placement
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The change adds an opt-in memory-level setting and localized optimization flags without any identified merge-blocking issue; it is merge-ready after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Victor-Jung
requested changes
Jul 8, 2026
Victor-Jung
left a comment
Victor-Jung
left a comment
Member
There was a problem hiding this comment.
I have some small comments but overall it looks good. I'd like to see the runtime overhead of moving these tiling information in L2 for a small layer. If the overhead is not so negligible we will add the option to chose where these tiling info are stored.
This reverts commit 1e91ff7.
Adds TilingHoistingMixIn.tileControlTableMemoryLevel. It defaults to None, which keeps the current behaviour of placing the hoisted tile-control tables in the tiled level, so nothing changes unless a platform opts in. Setting it to "L2" on GAP9 moves those tables away from the cluster master stack, where a deep stack write can corrupt a DMA cmd and hang mchan_transfer_wait() (seen on MobileNetV1 training). The tables are file-scope static arrays and never come out of the tile arena, so at a fixed --l1 the tiling solution, the DMA descriptors and the kernel code are unchanged either way: generating VisualWakeWords with and without the override gives identical Network.c apart from the PI_L1 / PI_L2 section attribute on 60 arrays.
runwangdl
force-pushed
the
fix-gap9-l3-board
branch
from
August 19, 2026 16:41
a2adfc6 to
3fe7c12
Compare
runwangdl
changed the title
Victor-Jung
approved these changes
Aug 20, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1.
-O3on the hot forward kernelsTargetLibraries/GAP9/CMakeLists.txt— compile Conv / DWConv / Gemm at-O3, appended last so itwins over the SDK's default
-Os. This turns on the RISC-V (XpulpV2) hardware loops on the tightinner loops.
2. Make the tile-control-table memory level configurable
Deeploy/TilingExtension/CodeTransformationPasses/TilingHoistingMixIn.py— addstileControlTableMemoryLevel, defaulting toNone:Noneis the current behaviour, so this is a no-op unless a platform opts in.The hoisted tables (
numTiles, DMAcmd/size/ stride, tile dims, base offsets) are theread-only lookup tables the controller uses to drive the tiling loop and program the DMAs. Setting
the knob to
"L2"on GAP9 moves them out of L1 TCDM, where they otherwise sit next to the clustermaster stack — a deep stack write can corrupt a DMA
cmdand hangmchan_transfer_wait()(seen onMobileNetV1 training).