[go] internal/bytealg: use NEON for compare_arm64.s large-input path

5 views
Skip to first unread message

Gerrit Bot (Gerrit)

unread,
Jun 5, 2026, 9:44:34 PM (4 days ago) Jun 5
to Mauri de Souza Meneguzzo, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gerrit Bot uploaded new patchset

Gerrit Bot uploaded patch set #3 to this change.
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newpatchset
Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I4ce9a26f7d434eb4f6447d8a5cbebee5d0901753
Gerrit-Change-Number: 786560
Gerrit-PatchSet: 3
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Mauri de Souza Meneguzzo <maur...@gmail.com>
unsatisfied_requirement
open
diffy

Mauri de Souza Meneguzzo (Gerrit)

unread,
Jun 5, 2026, 11:02:08 PM (4 days ago) Jun 5
to Gerrit Bot, goph...@pubsubhelper.golang.org, Keith Randall, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Keith Randall

Mauri de Souza Meneguzzo voted and added 1 comment

Votes added by Mauri de Souza Meneguzzo

Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 1:
Gopher Robot . resolved

I spotted some possible problems with your PR:

  1. You usually need to reference a bug number for all but trivial or cosmetic fixes. For this repo, the format is usually 'Fixes #12345' or 'Updates #12345' at the end of the commit message. Should you have a bug reference?

Please address any problems by updating the GitHub PR.

When complete, mark this comment as 'Done' and click the [blue 'Reply' button](https://go.dev/wiki/GerritBot#i-left-a-reply-to-a-comment-in-gerrit-but-no-one-but-me-can-see-it) above. These findings are based on heuristics; if a finding does not apply, briefly reply here saying so.

To update the commit title or commit message body shown here in Gerrit, you must edit the GitHub PR title and PR description (the first comment) in the GitHub web interface using the 'Edit' button or 'Edit' menu entry there. Note: pushing a new commit to the PR will not automatically update the commit message used by Gerrit.

For more details, see:

(In general for Gerrit code reviews, the change author is expected to [log in to Gerrit](https://go-review.googlesource.com/login/) with a Gmail or other Google account and then close out each piece of feedback by marking it as 'Done' if implemented as suggested or otherwise reply to each review comment. See the [Review](https://go.dev/doc/contribute#review) section of the Contributing Guide for details.)

Mauri de Souza Meneguzzo

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Keith Randall
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I4ce9a26f7d434eb4f6447d8a5cbebee5d0901753
    Gerrit-Change-Number: 786560
    Gerrit-PatchSet: 3
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Keith Randall <k...@golang.org>
    Gerrit-Reviewer: Mauri de Souza Meneguzzo <maur...@gmail.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Keith Randall <k...@golang.org>
    Gerrit-Comment-Date: Sat, 06 Jun 2026 03:01:47 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Gopher Robot <go...@golang.org>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Keith Randall (Gerrit)

    unread,
    Jun 9, 2026, 5:53:02 PM (11 hours ago) Jun 9
    to Gerrit Bot, Mauri de Souza Meneguzzo, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Keith Randall, Gopher Robot, golang-co...@googlegroups.com

    Keith Randall added 3 comments

    File src/internal/bytealg/compare_arm64.s
    Line 54, Patchset 3 (Latest): VCMEQ V0.D2, V4.D2, V8.D2
    Keith Randall . unresolved

    It seems very strange to use .D2 here instead of .B16. Is there a reason for that?

    Line 60, Patchset 3 (Latest): VAND V8.B16, V11.B16, V8.B16
    Keith Randall . unresolved

    Instead of ((V8&V9)&V10)&V11, do (V8&V9)&(V10&V11).

    Line 87, Patchset 3 (Parent): ADD $8, R0
    Keith Randall . unresolved

    This needs a comment, tricky things are happening.

    Part of the confusion is that the entry point `tail` needs a comment. It is not checking [R0:R0+R6] like everywhere else is. It is checking [R0+R6:R0+R6+8].

    Previously it was always expecting R6 to be negative. Now here we are coyly not incrementing R0 and passing a positive R6.

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I4ce9a26f7d434eb4f6447d8a5cbebee5d0901753
      Gerrit-Change-Number: 786560
      Gerrit-PatchSet: 3
      Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
      Gerrit-Reviewer: Keith Randall <k...@golang.org>
      Gerrit-Reviewer: Mauri de Souza Meneguzzo <maur...@gmail.com>
      Gerrit-CC: Gopher Robot <go...@golang.org>
      Gerrit-Comment-Date: Tue, 09 Jun 2026 21:52:58 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      unsatisfied_requirement
      satisfied_requirement
      open
      diffy

      Gerrit Bot (Gerrit)

      unread,
      Jun 9, 2026, 6:56:40 PM (10 hours ago) Jun 9
      to Mauri de Souza Meneguzzo, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
      Attention needed from Keith Randall and Mauri de Souza Meneguzzo

      Gerrit Bot uploaded new patchset

      Gerrit Bot uploaded patch set #4 to this change.
      Following approvals got outdated and were removed:
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Keith Randall
      • Mauri de Souza Meneguzzo
      Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: newpatchset
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I4ce9a26f7d434eb4f6447d8a5cbebee5d0901753
        Gerrit-Change-Number: 786560
        Gerrit-PatchSet: 4
        Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
        Gerrit-Reviewer: Keith Randall <k...@golang.org>
        Gerrit-CC: Mauri de Souza Meneguzzo <maur...@gmail.com>
        Gerrit-Attention: Keith Randall <k...@golang.org>
        Gerrit-Attention: Mauri de Souza Meneguzzo <maur...@gmail.com>
        unsatisfied_requirement
        open
        diffy

        Mauri de Souza Meneguzzo (Gerrit)

        unread,
        Jun 9, 2026, 6:57:21 PM (10 hours ago) Jun 9
        to Gerrit Bot, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Keith Randall, Gopher Robot, golang-co...@googlegroups.com
        Attention needed from Keith Randall and Mauri de Souza Meneguzzo

        Mauri de Souza Meneguzzo added 3 comments

        File src/internal/bytealg/compare_arm64.s
        Line 54, Patchset 3: VCMEQ V0.D2, V4.D2, V8.D2
        Keith Randall . unresolved

        It seems very strange to use .D2 here instead of .B16. Is there a reason for that?

        Mauri de Souza Meneguzzo

        Ehh, no good reason. That's just a mistake. .D2 zeroes an entire lane on a mismatch, while .B16 compares all bytes individually. Both happen to work here because the result is eventually reduced with VUMINV.B16, but .B16 is the right choice since we're doing a byte comparison in the first place.

        Line 60, Patchset 3: VAND V8.B16, V11.B16, V8.B16
        Keith Randall . unresolved

        Instead of ((V8&V9)&V10)&V11, do (V8&V9)&(V10&V11).

        Mauri de Souza Meneguzzo

        Thanks, didn't see the parallel opportunity there.

        Keith Randall . resolved

        This needs a comment, tricky things are happening.

        Part of the confusion is that the entry point `tail` needs a comment. It is not checking [R0:R0+R6] like everywhere else is. It is checking [R0+R6:R0+R6+8].

        Previously it was always expecting R6 to be negative. Now here we are coyly not incrementing R0 and passing a positive R6.

        Mauri de Souza Meneguzzo

        Acknowledged

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Keith Randall
        • Mauri de Souza Meneguzzo
        Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: comment
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I4ce9a26f7d434eb4f6447d8a5cbebee5d0901753
        Gerrit-Change-Number: 786560
        Gerrit-PatchSet: 3
        Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
        Gerrit-Reviewer: Keith Randall <k...@golang.org>
        Gerrit-CC: Gopher Robot <go...@golang.org>
        Gerrit-CC: Mauri de Souza Meneguzzo <maur...@gmail.com>
        Gerrit-Attention: Keith Randall <k...@golang.org>
        Gerrit-Attention: Mauri de Souza Meneguzzo <maur...@gmail.com>
        Gerrit-Comment-Date: Tue, 09 Jun 2026 22:57:17 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Keith Randall <k...@golang.org>
        unsatisfied_requirement
        open
        diffy

        Gerrit Bot (Gerrit)

        unread,
        Jun 9, 2026, 7:09:31 PM (10 hours ago) Jun 9
        to Mauri de Souza Meneguzzo, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
        Attention needed from Keith Randall and Mauri de Souza Meneguzzo

        Gerrit Bot uploaded new patchset

        Gerrit Bot uploaded patch set #5 to this change.
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Keith Randall
        • Mauri de Souza Meneguzzo
        Submit Requirements:
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • requirement is not satisfiedReview-Enforcement
        • requirement is not satisfiedTryBots-Pass
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: newpatchset
        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I4ce9a26f7d434eb4f6447d8a5cbebee5d0901753
        Gerrit-Change-Number: 786560
        Gerrit-PatchSet: 5
        unsatisfied_requirement
        open
        diffy
        Reply all
        Reply to author
        Forward
        0 new messages