Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: simple generics are not inlined #54497

Closed
piotr-sneller opened this issue Aug 17, 2022 · 4 comments
Closed

cmd/compile: simple generics are not inlined #54497

piotr-sneller opened this issue Aug 17, 2022 · 4 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@piotr-sneller
Copy link

piotr-sneller commented Aug 17, 2022

What version of Go are you using (go version)?

1.19

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

amd64/windows

Even the simplest one-line generic helpers are not inlined, which essentially kills the generic-based modularization approach. The compiler fails to inline and then strength-reduce the resulting code and generates the most generic variant instead. Not only will it severely impact performance, if the helper is called from the inner loop, but can blossom with all sorts of myths regarding Go generics, preventing their further adoption. The latter suggests the issue is more critical than it might initially appear.

What did you do?

package testcase

type C interface { ~uint | ~uint32 | ~uint64 }

func isAligned[T C](x, y T) bool {
    return x % y == 0
}

func foo(x uint) bool {
    return isAligned(x, 64)
}

Compile with:
go tool compile -S t1.go

What did you expect to see?

TEST $3f, AX

What did you see instead?

caller:

      0x0014 00020 (t1.go:11) MOVQ    AX, BX                                          
      0x0017 00023 (t1.go:11) MOVL    $64, CX                                         
      0x001c 00028 (t1.go:11) LEAQ    <unlinkable>..dict.isAligned[uint](SB), AX      
      0x0023 00035 (t1.go:11) PCDATA  $1, $0                                          
      0x0023 00035 (t1.go:11) CALL    <unlinkable>.isAligned[go.shape.uint_0](SB)     
      0x0028 00040 (t1.go:11) MOVQ    24(SP), BP                                      
      0x002d 00045 (t1.go:11) ADDQ    $32, SP                                         
      0x0031 00049 (t1.go:11) RET       

callee:

<unlinkable>.isAligned[go.shape.uint_0] STEXT dupok nosplit size=46 args=0x18 locals=0x8 funcid=0x0 align=0x0
        0x0000 00000 (t1.go:6)  TEXT    <unlinkable>.isAligned[go.shape.uint_0](SB), DUPOK|NOSPLIT|ABIInternal, $8-24
        0x0000 00000 (t1.go:6)  SUBQ    $8, SP
        0x0004 00004 (t1.go:6)  MOVQ    BP, (SP)
        0x0008 00008 (t1.go:6)  LEAQ    (SP), BP
        0x000c 00012 (t1.go:6)  FUNCDATA        $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
        0x000c 00012 (t1.go:6)  FUNCDATA        $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
        0x000c 00012 (t1.go:6)  FUNCDATA        $5, <unlinkable>.isAligned[go.shape.uint_0].arginfo1(SB)
        0x000c 00012 (t1.go:6)  FUNCDATA        $6, <unlinkable>.isAligned[go.shape.uint_0].argliveinfo(SB)
        0x000c 00012 (t1.go:6)  PCDATA  $3, $1
        0x000c 00012 (t1.go:7)  TESTQ   CX, CX
        0x000f 00015 (t1.go:7)  JEQ     40
        0x0011 00017 (t1.go:7)  MOVQ    BX, AX
        0x0014 00020 (t1.go:7)  XORL    DX, DX
        0x0016 00022 (t1.go:7)  DIVQ    CX
        0x0019 00025 (t1.go:7)  TESTQ   DX, DX
        0x001c 00028 (t1.go:7)  SETEQ   AL
        0x001f 00031 (t1.go:7)  MOVQ    (SP), BP
        0x0023 00035 (t1.go:7)  ADDQ    $8, SP
        0x0027 00039 (t1.go:7)  RET
        0x0028 00040 (t1.go:7)  PCDATA  $1, $0
        0x0028 00040 (t1.go:7)  CALL    runtime.panicdivide(SB)
@seankhliao seankhliao changed the title Simple generics are not inlined cmd/compile: simple generics are not inlined Aug 17, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 17, 2022
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. and removed compiler/runtime Issues related to the Go compiler and/or runtime. labels Aug 17, 2022
@randall77
Copy link
Contributor

randall77 commented Aug 17, 2022

This will be fixed with unified IR. Inlining in this case was disabled to fix a bug. See CL 395854. Closing as a dup of that issue.

@randall77
Copy link
Contributor

randall77 commented Aug 17, 2022

Actually, not closed, but will be fixed soon.

@randall77 randall77 reopened this Aug 17, 2022
@randall77 randall77 added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 17, 2022
@randall77 randall77 added this to the Go1.20 milestone Aug 17, 2022
@mdempsky
Copy link
Member

mdempsky commented Aug 17, 2022

I think the issue is we severely limited inlining after having so many issues with inlining, shape types, and the non-unified frontend:

// Don't inline a function fn that has no shape parameters, but is passed at

I'll look into getting this working with unified IR after CL 421821 lands. I think that code probably just needs to be skipped for unified IR mode.

@gopherbot
Copy link

gopherbot commented Aug 18, 2022

Change https://go.dev/cl/424775 mentions this issue: cmd/compile: enable more inlining for unified IR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
Status: Done
Development

No branches or pull requests

5 participants