[go] cmd/compile: use ABI0 for cgo_unsafe_args functions

92 views
Skip to first unread message

Cherry Zhang (Gerrit)

unread,
Apr 1, 2021, 11:14:46 AM4/1/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Cherry Zhang has uploaded this change for review.

View Change

cmd/compile: use ABI0 for cgo_unsafe_args functions

cgo_unsafe_args paragma indicates that the function (or its
callee) uses address and offsets to dispatch arguments, which
currently using ABI0 frame layout. Pin them to ABI0.

With this, "GOEXPERIMENT=regabi,regabiargs go run hello.go" works
on Darwin/AMD64.

Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
---
M src/cmd/compile/internal/ssagen/ssa.go
1 file changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index 9c1c493..22993fe 100644
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -232,6 +232,12 @@
if fn == nil {
return abi1
}
+ if fn.Pragma&ir.CgoUnsafeArgs != 0 {
+ // CgoUnsafeArgs indicates the function (or its callee) uses
+ // offsets to dispatch arguments, which currently using ABI0
+ // frame layout. Pin it to ABI0.
+ return abi0
+ }
switch fn.ABI {
case obj.ABI0:
return abi0

To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
Gerrit-Change-Number: 306609
Gerrit-PatchSet: 1
Gerrit-Owner: Cherry Zhang <cher...@google.com>
Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
Gerrit-MessageType: newchange

Cherry Zhang (Gerrit)

unread,
Apr 1, 2021, 11:15:05 AM4/1/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Patch set 1:Run-TryBot +1

View Change

    To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
    Gerrit-Change-Number: 306609
    Gerrit-PatchSet: 1
    Gerrit-Owner: Cherry Zhang <cher...@google.com>
    Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
    Gerrit-Comment-Date: Thu, 01 Apr 2021 15:15:00 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Than McIntosh (Gerrit)

    unread,
    Apr 1, 2021, 11:39:54 AM4/1/21
    to Cherry Zhang, goph...@pubsubhelper.golang.org, Go Bot, Austin Clements, golang-co...@googlegroups.com

    Attention is currently required from: Austin Clements, Cherry Zhang.

    Patch set 1:Code-Review +2

    View Change

      To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      Gerrit-Change-Number: 306609
      Gerrit-PatchSet: 1
      Gerrit-Owner: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Austin Clements <aus...@google.com>
      Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Than McIntosh <th...@google.com>
      Gerrit-Attention: Austin Clements <aus...@google.com>
      Gerrit-Attention: Cherry Zhang <cher...@google.com>
      Gerrit-Comment-Date: Thu, 01 Apr 2021 15:39:49 +0000

      Cherry Zhang (Gerrit)

      unread,
      Apr 1, 2021, 1:12:10 PM4/1/21
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Austin Clements, Cherry Zhang.

      Cherry Zhang uploaded patch set #2 to this change.

      View Change

      cmd/compile: use ABI0 for cgo_unsafe_args functions

      cgo_unsafe_args paragma indicates that the function (or its
      callee) uses address and offsets to dispatch arguments, which
      currently using ABI0 frame layout. Pin them to ABI0.

      With this, "GOEXPERIMENT=regabi,regabiargs go run hello.go" works
      on Darwin/AMD64.

      Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      ---
      M src/cmd/compile/internal/amd64/ssa.go
      M src/cmd/compile/internal/ssagen/abi.go
      2 files changed, 13 insertions(+), 4 deletions(-)

      To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      Gerrit-Change-Number: 306609
      Gerrit-PatchSet: 2
      Gerrit-Owner: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Austin Clements <aus...@google.com>
      Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Than McIntosh <th...@google.com>
      Gerrit-Attention: Austin Clements <aus...@google.com>
      Gerrit-Attention: Cherry Zhang <cher...@google.com>
      Gerrit-MessageType: newpatchset

      Cherry Zhang (Gerrit)

      unread,
      Apr 1, 2021, 1:12:44 PM4/1/21
      to goph...@pubsubhelper.golang.org, Than McIntosh, Go Bot, Austin Clements, golang-co...@googlegroups.com

      Attention is currently required from: Austin Clements.

      Patch set 2:Run-TryBot +1

      View Change

      1 comment:

      To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      Gerrit-Change-Number: 306609
      Gerrit-PatchSet: 2
      Gerrit-Owner: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Austin Clements <aus...@google.com>
      Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Than McIntosh <th...@google.com>
      Gerrit-Attention: Austin Clements <aus...@google.com>
      Gerrit-Comment-Date: Thu, 01 Apr 2021 17:12:39 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Gerrit-MessageType: comment

      Austin Clements (Gerrit)

      unread,
      Apr 1, 2021, 3:17:28 PM4/1/21
      to Cherry Zhang, goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

      Attention is currently required from: Cherry Zhang.

      View Change

      3 comments:

      • File src/cmd/compile/internal/amd64/ssa.go:

        • Patch Set #2, Line 828: // XXX s.ABI != obj.ABIInternal can only happen for ABI wrappers

          Ah, interesting... I know this condition currently protects several things in this file, and this kind of suggests to me that the condition needs to be different everywhere. It seems like it was just luck that we weren't hitting this path before in ABI0 functions and I don't think we want to depend on that anywhere else. Maybe everywhere we use X15 needs to be protected by both the current function's ABI and the experiment?

        • Patch Set #2, Line 830:

          For cgo_unsafe_args functions it is actually Go
          // function so X15 should already be zero.

          This is also interesting. I'm okay with thinking of cgo_unsafe_args functions as "no, really, these are ABI0 functions" even though they could theoretically be somewhere in-between. Mostly I worry about introducing a third case that's kind of ABI0 and kind of ABIInternal but doesn't get exercised that much, particularly if/when we start shifting ABIInternal further away from ABI0.

      • File src/cmd/compile/internal/ssagen/abi.go:

      To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      Gerrit-Change-Number: 306609
      Gerrit-PatchSet: 2
      Gerrit-Owner: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Austin Clements <aus...@google.com>
      Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Than McIntosh <th...@google.com>
      Gerrit-Attention: Cherry Zhang <cher...@google.com>
      Gerrit-Comment-Date: Thu, 01 Apr 2021 19:17:23 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Gerrit-MessageType: comment

      Cherry Zhang (Gerrit)

      unread,
      Apr 1, 2021, 5:10:56 PM4/1/21
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Cherry Zhang.

      Cherry Zhang uploaded patch set #3 to this change.

      View Change

      cmd/compile: use ABI0 for cgo_unsafe_args functions

      cgo_unsafe_args paragma indicates that the function (or its
      callee) uses address and offsets to dispatch arguments, which
      currently using ABI0 frame layout. Pin them to ABI0.

      With this, "GOEXPERIMENT=regabi,regabiargs go run hello.go" works
      on Darwin/AMD64.

      Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      ---
      M src/cmd/compile/internal/amd64/ssa.go
      M src/cmd/compile/internal/ssagen/abi.go
      2 files changed, 9 insertions(+), 8 deletions(-)

      To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      Gerrit-Change-Number: 306609
      Gerrit-PatchSet: 3
      Gerrit-Owner: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Austin Clements <aus...@google.com>
      Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Than McIntosh <th...@google.com>
      Gerrit-Attention: Cherry Zhang <cher...@google.com>
      Gerrit-MessageType: newpatchset

      Cherry Zhang (Gerrit)

      unread,
      Apr 1, 2021, 5:11:29 PM4/1/21
      to goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

      Attention is currently required from: Austin Clements.

      View Change

      4 comments:

      • Patchset:

      • File src/cmd/compile/internal/amd64/ssa.go:

        • I don't think it was just luck. Before, ABI0 functions being compiled are just wrappers, which have a very specific structure and I don't think they contain zeroing ops. That's why I wrote the condition.

          We do need to do this for DUFFZERO also.

        • This is also interesting. […]

          Okay, just make them ABI0 functions, then. Dropped the comment.

      • File src/cmd/compile/internal/ssagen/abi.go:

        • Maybe move the CgoUnsafeArgs check up here with the other fn. […]

          Done

      To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: go
      Gerrit-Branch: master
      Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
      Gerrit-Change-Number: 306609
      Gerrit-PatchSet: 3
      Gerrit-Owner: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Austin Clements <aus...@google.com>
      Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
      Gerrit-Reviewer: Go Bot <go...@golang.org>
      Gerrit-Reviewer: Than McIntosh <th...@google.com>
      Gerrit-Attention: Austin Clements <aus...@google.com>
      Gerrit-Comment-Date: Thu, 01 Apr 2021 21:11:24 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Austin Clements <aus...@google.com>
      Gerrit-MessageType: comment

      Cherry Zhang (Gerrit)

      unread,
      Apr 1, 2021, 5:11:38 PM4/1/21
      to goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

      Attention is currently required from: Austin Clements.

      Patch set 3:Run-TryBot +1

      View Change

        To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
        Gerrit-Change-Number: 306609
        Gerrit-PatchSet: 3
        Gerrit-Owner: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Austin Clements <aus...@google.com>
        Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Go Bot <go...@golang.org>
        Gerrit-Reviewer: Than McIntosh <th...@google.com>
        Gerrit-Attention: Austin Clements <aus...@google.com>
        Gerrit-Comment-Date: Thu, 01 Apr 2021 21:11:34 +0000

        Austin Clements (Gerrit)

        unread,
        Apr 1, 2021, 6:04:34 PM4/1/21
        to Cherry Zhang, goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

        Attention is currently required from: Cherry Zhang.

        View Change

        2 comments:

        • Patchset:

        • File src/cmd/compile/internal/amd64/ssa.go:

          • Patch Set #3, Line 1001: if objabi.Experiment.RegabiG {

            Should we also guard this with s.ABI so this can appear in ABI0 functions? Or do you think we should just eliminate LoweredGetG when we commit to regabig?

        To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
        Gerrit-Change-Number: 306609
        Gerrit-PatchSet: 3
        Gerrit-Owner: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Austin Clements <aus...@google.com>
        Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Go Bot <go...@golang.org>
        Gerrit-Reviewer: Than McIntosh <th...@google.com>
        Gerrit-Attention: Cherry Zhang <cher...@google.com>
        Gerrit-Comment-Date: Thu, 01 Apr 2021 22:04:30 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Gerrit-MessageType: comment

        Cherry Zhang (Gerrit)

        unread,
        Apr 1, 2021, 6:31:55 PM4/1/21
        to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

        Attention is currently required from: Cherry Zhang.

        Cherry Zhang uploaded patch set #4 to this change.

        View Change

        cmd/compile: use ABI0 for cgo_unsafe_args functions

        cgo_unsafe_args paragma indicates that the function (or its
        callee) uses address and offsets to dispatch arguments, which
        currently using ABI0 frame layout. Pin them to ABI0.

        With this, "GOEXPERIMENT=regabi,regabiargs go run hello.go" works
        on Darwin/AMD64.

        Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
        ---
        M src/cmd/compile/internal/amd64/ssa.go
        M src/cmd/compile/internal/ssa/gen/AMD64.rules
        M src/cmd/compile/internal/ssa/rewriteAMD64.go
        M src/cmd/compile/internal/ssagen/abi.go
        4 files changed, 14 insertions(+), 12 deletions(-)

        To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
        Gerrit-Change-Number: 306609
        Gerrit-PatchSet: 4
        Gerrit-Owner: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Austin Clements <aus...@google.com>
        Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Go Bot <go...@golang.org>
        Gerrit-Reviewer: Than McIntosh <th...@google.com>
        Gerrit-Attention: Cherry Zhang <cher...@google.com>
        Gerrit-MessageType: newpatchset

        Cherry Zhang (Gerrit)

        unread,
        Apr 1, 2021, 6:32:31 PM4/1/21
        to goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

        Attention is currently required from: Austin Clements.

        View Change

        1 comment:

        • File src/cmd/compile/internal/amd64/ssa.go:

          • Should we also guard this with s. […]

            Good point. I guess we should guard this, and also the rewriting rule that lowers GetG. Done.

        To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: go
        Gerrit-Branch: master
        Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
        Gerrit-Change-Number: 306609
        Gerrit-PatchSet: 4
        Gerrit-Owner: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Austin Clements <aus...@google.com>
        Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
        Gerrit-Reviewer: Go Bot <go...@golang.org>
        Gerrit-Reviewer: Than McIntosh <th...@google.com>
        Gerrit-Attention: Austin Clements <aus...@google.com>
        Gerrit-Comment-Date: Thu, 01 Apr 2021 22:32:26 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No

        Cherry Zhang (Gerrit)

        unread,
        Apr 1, 2021, 7:29:35 PM4/1/21
        to goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

        Attention is currently required from: Austin Clements.

        Patch set 5:Run-TryBot +1

        View Change

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 5
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-Attention: Austin Clements <aus...@google.com>
          Gerrit-Comment-Date: Thu, 01 Apr 2021 23:29:31 +0000

          Cherry Zhang (Gerrit)

          unread,
          Apr 1, 2021, 7:40:18 PM4/1/21
          to goph...@pubsubhelper.golang.org, Go Bot, Than McIntosh, Austin Clements, golang-co...@googlegroups.com

          Attention is currently required from: Austin Clements.

          View Change

          1 comment:

          • Patchset:

            • The Windows failure is interesting. syscall.loadlibrary is a function defined in runtime and linkname'd to the syscall package. It is also cgo_unsafe_args. With this CL, in the runtime it gets an ABI0 TEXT symbol. In the syscall package, there is a Go declaration (bodyless), Go reference, and a dummy assembly reference. Somehow this triggers generation of ABI alias, so it gets an ABI0 ABIALIAS symbol. The two ABI0 symbols confuses the linker.

              CL 306709 removes the dummy assembly reference, and it makes it work. (I don't see why we need the dummy reference.)

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 5
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-Attention: Austin Clements <aus...@google.com>
          Gerrit-Comment-Date: Thu, 01 Apr 2021 23:40:13 +0000

          Austin Clements (Gerrit)

          unread,
          Apr 2, 2021, 12:17:27 PM4/2/21
          to Cherry Zhang, goph...@pubsubhelper.golang.org, Austin Clements, Go Bot, Than McIntosh, golang-co...@googlegroups.com

          Attention is currently required from: Cherry Zhang.

          Patch set 5:Code-Review +2

          View Change

          1 comment:

          • File src/cmd/compile/internal/amd64/ssa.go:

            • Patch Set #5, Line 1002: new ABI

              While we're here, "new ABI" -> "ABIInternal"? (It's "new" today but won't be in a few years :)

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 5
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-Attention: Cherry Zhang <cher...@google.com>
          Gerrit-Comment-Date: Fri, 02 Apr 2021 16:17:22 +0000

          Cherry Zhang (Gerrit)

          unread,
          Apr 2, 2021, 12:29:25 PM4/2/21
          to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

          Attention is currently required from: Cherry Zhang.

          Cherry Zhang uploaded patch set #6 to this change.

          View Change

          cmd/compile: use ABI0 for cgo_unsafe_args functions

          cgo_unsafe_args paragma indicates that the function (or its
          callee) uses address and offsets to dispatch arguments, which
          currently using ABI0 frame layout. Pin them to ABI0.

          With this, "GOEXPERIMENT=regabi,regabiargs go run hello.go" works
          on Darwin/AMD64.

          Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          ---
          M src/cmd/compile/internal/amd64/ssa.go
          M src/cmd/compile/internal/ssa/gen/AMD64.rules
          M src/cmd/compile/internal/ssa/rewriteAMD64.go
          M src/cmd/compile/internal/ssagen/abi.go
          4 files changed, 15 insertions(+), 13 deletions(-)

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 6
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-Attention: Cherry Zhang <cher...@google.com>
          Gerrit-MessageType: newpatchset

          Cherry Zhang (Gerrit)

          unread,
          Apr 2, 2021, 12:29:58 PM4/2/21
          to goph...@pubsubhelper.golang.org, Austin Clements, Go Bot, Than McIntosh, golang-co...@googlegroups.com

          View Change

          1 comment:

          • File src/cmd/compile/internal/amd64/ssa.go:

            • Patch Set #5, Line 1002: new ABI

              While we're here, "new ABI" -> "ABIInternal"? (It's "new" today but won't be in a few years :)

            • Done

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 6
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-Comment-Date: Fri, 02 Apr 2021 16:29:54 +0000

          Cherry Zhang (Gerrit)

          unread,
          Apr 2, 2021, 12:31:51 PM4/2/21
          to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Austin Clements, Go Bot, Than McIntosh, golang-co...@googlegroups.com

          Cherry Zhang submitted this change.

          View Change

          Approvals: Austin Clements: Looks good to me, approved Than McIntosh: Looks good to me, approved Cherry Zhang: Trusted
          cmd/compile: use ABI0 for cgo_unsafe_args functions

          cgo_unsafe_args paragma indicates that the function (or its
          callee) uses address and offsets to dispatch arguments, which
          currently using ABI0 frame layout. Pin them to ABI0.

          With this, "GOEXPERIMENT=regabi,regabiargs go run hello.go" works
          on Darwin/AMD64.

          Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Reviewed-on: https://go-review.googlesource.com/c/go/+/306609
          Trust: Cherry Zhang <cher...@google.com>
          Reviewed-by: Austin Clements <aus...@google.com>
          Reviewed-by: Than McIntosh <th...@google.com>

          ---
          M src/cmd/compile/internal/amd64/ssa.go
          M src/cmd/compile/internal/ssa/gen/AMD64.rules
          M src/cmd/compile/internal/ssa/rewriteAMD64.go
          M src/cmd/compile/internal/ssagen/abi.go
          4 files changed, 15 insertions(+), 13 deletions(-)

          diff --git a/src/cmd/compile/internal/amd64/ssa.go b/src/cmd/compile/internal/amd64/ssa.go
          index 2c767d3..8142ba7 100644
          --- a/src/cmd/compile/internal/amd64/ssa.go
          +++ b/src/cmd/compile/internal/amd64/ssa.go
          @@ -824,10 +824,7 @@
          p.To.Reg = v.Args[0].Reg()
          ssagen.AddAux2(&p.To, v, sc.Off64())
          case ssa.OpAMD64MOVOstorezero:
          - if s.ABI != obj.ABIInternal {
          - v.Fatalf("MOVOstorezero can be only used in ABIInternal functions")
          - }
          - if !objabi.Experiment.RegabiG {
          + if !objabi.Experiment.RegabiG || s.ABI != obj.ABIInternal {
          // zero X15 manually
          opregreg(s, x86.AXORPS, x86.REG_X15, x86.REG_X15)
          }
          @@ -918,10 +915,7 @@
          p.To.Type = obj.TYPE_REG
          p.To.Reg = v.Reg()
          case ssa.OpAMD64DUFFZERO:
          - if s.ABI != obj.ABIInternal {
          - v.Fatalf("MOVOconst can be only used in ABIInternal functions")
          - }
          - if !objabi.Experiment.RegabiG {
          + if !objabi.Experiment.RegabiG || s.ABI != obj.ABIInternal {
          // zero X15 manually
          opregreg(s, x86.AXORPS, x86.REG_X15, x86.REG_X15)
          }
          @@ -1004,8 +998,8 @@
          // Closure pointer is DX.
          ssagen.CheckLoweredGetClosurePtr(v)
          case ssa.OpAMD64LoweredGetG:
          - if objabi.Experiment.RegabiG {
          - v.Fatalf("LoweredGetG should not appear in new ABI")
          + if objabi.Experiment.RegabiG && s.ABI == obj.ABIInternal {
          + v.Fatalf("LoweredGetG should not appear in ABIInternal")
          }
          r := v.Reg()
          getgFromTLS(s, r)
          diff --git a/src/cmd/compile/internal/ssa/gen/AMD64.rules b/src/cmd/compile/internal/ssa/gen/AMD64.rules
          index 98cd865..839d4a3 100644
          --- a/src/cmd/compile/internal/ssa/gen/AMD64.rules
          +++ b/src/cmd/compile/internal/ssa/gen/AMD64.rules
          @@ -460,7 +460,7 @@
          (IsInBounds idx len) => (SETB (CMPQ idx len))
          (IsSliceInBounds idx len) => (SETBE (CMPQ idx len))
          (NilCheck ...) => (LoweredNilCheck ...)
          -(GetG mem) && !objabi.Experiment.RegabiG => (LoweredGetG mem) // only lower in old ABI. in new ABI we have a G register.
          +(GetG mem) && !(objabi.Experiment.RegabiG && v.Block.Func.OwnAux.Fn.ABI() == obj.ABIInternal) => (LoweredGetG mem) // only lower in old ABI. in new ABI we have a G register.
          (GetClosurePtr ...) => (LoweredGetClosurePtr ...)
          (GetCallerPC ...) => (LoweredGetCallerPC ...)
          (GetCallerSP ...) => (LoweredGetCallerSP ...)
          diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go
          index ce94fdb..1f56b70 100644
          --- a/src/cmd/compile/internal/ssa/rewriteAMD64.go
          +++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go
          @@ -4,6 +4,7 @@
          package ssa

          import "math"
          +import "cmd/internal/obj"
          import "cmd/internal/objabi"
          import "cmd/compile/internal/types"

          @@ -30465,11 +30466,11 @@
          func rewriteValueAMD64_OpGetG(v *Value) bool {
          v_0 := v.Args[0]
          // match: (GetG mem)
          - // cond: !objabi.Experiment.RegabiG
          + // cond: !(objabi.Experiment.RegabiG && v.Block.Func.OwnAux.Fn.ABI() == obj.ABIInternal)
          // result: (LoweredGetG mem)
          for {
          mem := v_0
          - if !(!objabi.Experiment.RegabiG) {
          + if !(!(objabi.Experiment.RegabiG && v.Block.Func.OwnAux.Fn.ABI() == obj.ABIInternal)) {
          break
          }
          v.reset(OpAMD64LoweredGetG)
          diff --git a/src/cmd/compile/internal/ssagen/abi.go b/src/cmd/compile/internal/ssagen/abi.go
          index 9c20383..61d065c 100644
          --- a/src/cmd/compile/internal/ssagen/abi.go
          +++ b/src/cmd/compile/internal/ssagen/abi.go
          @@ -146,6 +146,13 @@
          fn.ABI = defABI

          }

          + if fn.Pragma&ir.CgoUnsafeArgs != 0 {
          + // CgoUnsafeArgs indicates the function (or its callee) uses
          + // offsets to dispatch arguments, which currently using ABI0
          + // frame layout. Pin it to ABI0.
          +			fn.ABI = obj.ABI0
          + }
          +
          // Apply references.
          if abis, ok := s.refs[symName]; ok {
          fn.ABIRefs |= abis

          5 is the latest approved patch-set. The change was submitted with unreviewed changes in the following files: The name of the file: src/cmd/compile/internal/amd64/ssa.go Insertions: 1, Deletions: 1. ``` @@ -1001:1002, +1001:1002 @@ - v.Fatalf("LoweredGetG should not appear in new ABI") + v.Fatalf("LoweredGetG should not appear in ABIInternal") ```

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 7
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-MessageType: merged

          Cherry Zhang (Gerrit)

          unread,
          Apr 2, 2021, 12:31:51 PM4/2/21
          to goph...@pubsubhelper.golang.org, Austin Clements, Go Bot, Than McIntosh, golang-co...@googlegroups.com

          View Change

          1 comment:

          To view, visit change 306609. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: go
          Gerrit-Branch: master
          Gerrit-Change-Id: I3eadd5a3646a9de8fa681fa0a7f46e7cdc217d24
          Gerrit-Change-Number: 306609
          Gerrit-PatchSet: 6
          Gerrit-Owner: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Austin Clements <aus...@google.com>
          Gerrit-Reviewer: Cherry Zhang <cher...@google.com>
          Gerrit-Reviewer: Go Bot <go...@golang.org>
          Gerrit-Reviewer: Than McIntosh <th...@google.com>
          Gerrit-Comment-Date: Fri, 02 Apr 2021 16:31:44 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          Gerrit-MessageType: comment
          Reply all
          Reply to author
          Forward
          0 new messages