refactor mips loongArch packing coding style#6745
Conversation
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ae9a6ef03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6745 +/- ##
==========================================
- Coverage 95.43% 95.12% -0.31%
==========================================
Files 944 944
Lines 410856 409767 -1089
==========================================
- Hits 392094 389805 -2289
- Misses 18762 19962 +1200 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Refactors the MIPS and LoongArch packing layer code to simplify nested #if preprocessor guards into a flatter style, and aligns the bf16 elempack-selection branches in testutil.cpp, perfutil.cpp, and net.cpp to a uniform per-platform #elif structure. Adds a new test_slice_4 covering slice configurations whose sliced sizes mix % 8 == 0 and non-multiple-of-8 parts (exercising the elempack-selection logic touched in this PR).
Changes:
- In
packing_loongarch.cpp, unconditionally declarepack1to8/pack8to1/pack4to8/pack8to4and move__loongarch_sx/__loongarch_asxguards down to wrap only the SIMD inner loops; remove a few small LSX/LASX intrinsic fast paths in favor of scalar code. - In
packing_mips.cpp, replace#if __mips_msaguarded MSA fast-paths for two bf16 pack8to4 inner loops with scalar implementations. - In
net.cpp,tests/testutil.cpp, andtests/perf/perfutil.cpp, split the combinedNCNN_LASX || NCNN_LSX16-bit elempack-resolution block into separate#elif NCNN_LASX/#elif NCNN_LSXbranches and normalize the bf16-storage condition ordering. - Add
test_slice_4totests/test_slice.cpp.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/layer/loongarch/packing_loongarch.cpp | Flatten nested #if __loongarch_sx/_asx guards; drop a few LSX/LASX intrinsic paths for pack4to8/pack8to4 in favor of scalar loops. |
| src/layer/mips/packing_mips.cpp | Remove MSA SIMD fast path in two bf16 pack8to4 inner loops, leaving the scalar implementation. |
| src/net.cpp | Split combined `NCNN_LASX |
| tests/testutil.cpp | Same refactor of the 16-bit elempack-selection branches as in net.cpp. |
| tests/perf/perfutil.cpp | Same refactor of the 16-bit elempack-selection branches as in net.cpp. |
| tests/test_slice.cpp | Adds test_slice_4 exercising slice indices that mix %8==0 and non-multiple-of-8 chunk sizes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c61e7dbb5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
No description provided.