From 905ff3ad4f782d62207a5f14e9c93d72ba18723c Mon Sep 17 00:00:00 2001 From: Gerrit User 1115898 <1115898@3ce6091f-6c88-37e8-8c75-72f92ae8dfba> Date: Tue, 27 Oct 2026 15:52:53 -0700 Subject: [PATCH] Update patch set 7 Patch Set 7: (5 comments) Addressed all review comments in the latest patchset. Patch-set: 7 --- 1b1216abb649754576866b54f03a5229a02d9e52 | 90 ++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/1b1216abb649754576866b54f03a5229a02d9e52 b/1b1216abb649754576866b54f03a5229a02d9e52 index 0c22d9ab4..3ff32822c 100644 --- a/1b1216abb649754576866b54f03a5229a02d9e52 +++ b/1b1216abb649754576866b54f03a5229a02d9e52 @@ -34,6 +34,24 @@ "revId": "1b1216abb649754576866b54f03a5229a02d9e52", "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" }, + { + "unresolved": false, + "key": { + "uuid": "837a75a9_ebeae93e", + "filename": "source/planar_functions.cc", + "patchSetId": 7 + }, + "lineNbr": 2936, + "author": { + "id": 1115898 + }, + "writtenOn": "2026-10-27T22:52:53Z", + "side": 1, + "message": "Done. Removed the unreachable #elif defined(HAS_MIRRORUVROW_NEON) branch. Keeping dedicated 16-bit element row functions (MirrorRow_16_* and CopyRow_16_*) allows tail masking to be done with 16-bit predicates or rep movsw/stosw.", + "parentUuid": "ef108627_f67f97d7", + "revId": "1b1216abb649754576866b54f03a5229a02d9e52", + "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" + }, { "unresolved": true, "key": { @@ -51,6 +69,24 @@ "revId": "1b1216abb649754576866b54f03a5229a02d9e52", "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" }, + { + "unresolved": false, + "key": { + "uuid": "ff8f5d5e_04637c87", + "filename": "source/row_neon.cc", + "patchSetId": 7 + }, + "lineNbr": 3995, + "author": { + "id": 1115898 + }, + "writtenOn": "2026-10-27T22:52:53Z", + "side": 1, + "message": "Done. Updated CopyPlane_16 in source/planar_functions.cc to dispatch CopyRow_16 (including CopyRow_16_NEON / CopyRow_16_Any_NEON and CopyRow_16_HVX) and added CopyPlane_16 unit tests in unit_test/planar_test.cc. Keeping dedicated 16-bit element functions (CopyRow_16_* and MirrorRow_16_*) allows tail masking to be done with 16-bit predicates or rep movsw/stosw.", + "parentUuid": "393b10e1_4fa315af", + "revId": "1b1216abb649754576866b54f03a5229a02d9e52", + "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" + }, { "unresolved": true, "key": { @@ -68,6 +104,24 @@ "revId": "1b1216abb649754576866b54f03a5229a02d9e52", "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" }, + { + "unresolved": false, + "key": { + "uuid": "9d802295_65c69e9a", + "filename": "source/row_neon.cc", + "patchSetId": 7 + }, + "lineNbr": 4188, + "author": { + "id": 1115898 + }, + "writtenOn": "2026-10-27T22:52:53Z", + "side": 1, + "message": "Done. Replaced vmov + vmla.f32 with vmul.f32 + vadd.f32 to avoid the two vmov instructions per iteration.", + "parentUuid": "cb6aa027_8a0f308c", + "revId": "1b1216abb649754576866b54f03a5229a02d9e52", + "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" + }, { "unresolved": true, "key": { @@ -85,6 +139,24 @@ "revId": "1b1216abb649754576866b54f03a5229a02d9e52", "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" }, + { + "unresolved": false, + "key": { + "uuid": "337fea1f_c5575252", + "filename": "source/row_neon64.cc", + "patchSetId": 7 + }, + "lineNbr": 5927, + "author": { + "id": 1115898 + }, + "writtenOn": "2026-10-27T22:52:53Z", + "side": 1, + "message": "We benchmarked both on Apple M6: packing all 4 bytes with 3 bfi instructions and a single post-indexed str %w5, [%0], #4 took 8.24 ms (vs 8.17 ms for C) due to the serial bfi dependency chain waiting on all 4 table loads, whereas strh + strb + strb + add took 7.43 ms (1.10x faster than C) since each store depends on at most one ALU op after its load.", + "parentUuid": "f2aed825_a3572d3a", + "revId": "1b1216abb649754576866b54f03a5229a02d9e52", + "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" + }, { "unresolved": true, "key": { @@ -101,6 +173,24 @@ "message": "Two small notes here:\n1. Like the SSE2 block above, this calls `ARGBAffineRow_NEON` unconditionally before checking `TestCpuFlag(kCpuHasNEON)` on line 1583.\n2. This only tests `width \u003d 1280` (a multiple of 4) and `src_argb_stride \u003d 0`, so the 1-3 pixel tail loop and non-zero `src_argb_stride` path in `ARGBAffineRow_NEON` aren\u0027t exercised.", "revId": "1b1216abb649754576866b54f03a5229a02d9e52", "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" + }, + { + "unresolved": false, + "key": { + "uuid": "46373fbe_f81b14e2", + "filename": "unit_test/planar_test.cc", + "patchSetId": 7 + }, + "lineNbr": 1579, + "author": { + "id": 1115898 + }, + "writtenOn": "2026-10-27T22:52:53Z", + "side": 1, + "message": "Done. Guarded both SSE2 and NEON calls with TestCpuFlag checks and added a width \u003d 1279 test case with non-zero src_argb_stride to exercise both the 1-3 pixel tail loop and stride arithmetic.", + "parentUuid": "32ca45d5_15db4360", + "revId": "1b1216abb649754576866b54f03a5229a02d9e52", + "serverId": "3ce6091f-6c88-37e8-8c75-72f92ae8dfba" } ] } \ No newline at end of file