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: optimize reflect.TypeOf(x)==reflect.TypeOf(y) #70738

Open
adonovan opened this issue Dec 9, 2024 · 2 comments
Open

cmd/compile: optimize reflect.TypeOf(x)==reflect.TypeOf(y) #70738

adonovan opened this issue Dec 9, 2024 · 2 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Dec 9, 2024

I notice that the code generated for a comparison of rtypes is surprisingly poor, involving a ton of spills and a call to an intrinsic:

func sameType(x, y any) bool {
	return reflect.TypeOf(x) != reflect.TypeOf(y)
}
golang.org/x/tools/gopls/internal/golang.sameType STEXT size=176 args=0x20 locals=0x48 funcid=0x0 align=0x0
	0x0000 00000 	L0505	TEXT	golang.org/x/tools/gopls/internal/golang.sameType(SB), ABIInternal, $80-32
	0x0000 00000 	L0505	MOVD	16(g), R16
	0x0004 00004 	L0505	PCDATA	$0, $-2
	0x0004 00004 	L0505	CMP	R16, RSP
	0x0008 00008 	L0505	BLS	144
	0x000c 00012 	L0505	PCDATA	$0, $-1
	0x000c 00012 	L0505	MOVD.W	R30, -80(RSP)
	0x0010 00016 	L0505	MOVD	R29, -8(RSP)
	0x0014 00020 	L0505	SUB	$8, RSP, R29
	0x0018 00024 	L0505	MOVD	R0, golang.org/x/tools/gopls/internal/golang.x(FP)
	0x001c 00028 	L0505	MOVD	R1, golang.org/x/tools/gopls/internal/golang.x+8(FP)
	0x0020 00032 	L0505	MOVD	R2, golang.org/x/tools/gopls/internal/golang.y+16(FP)
	0x0024 00036 	L0505	MOVD	R3, golang.org/x/tools/gopls/internal/golang.y+24(FP)
	0x0028 00040 	L0505	FUNCDATA	$0, gclocals·emgMVsd5mo9g0HGy8lQYQA==(SB)
	0x0028 00040 	L0505	FUNCDATA	$1, gclocals·lV4l69IdnTMCVxlqSaJ25Q==(SB)
	0x0028 00040 	L0505	FUNCDATA	$2, golang.org/x/tools/gopls/internal/golang.sameType.stkobj(SB)
	0x0028 00040 	L0505	FUNCDATA	$5, golang.org/x/tools/gopls/internal/golang.sameType.arginfo1(SB)
	0x0028 00040 	L0505	FUNCDATA	$6, golang.org/x/tools/gopls/internal/golang.sameType.argliveinfo(SB)
	0x0028 00040 	L0505	PCDATA	$3, $1
	0x0028 00040 	     	NOP
	0x0028 00040 	L1303	MOVD	R0, internal/abi.a-16(SP)
	0x002c 00044 	L1303	MOVD	R1, internal/abi.a-8(SP)
	0x0030 00048 	L0179	MOVD	internal/abi.a-16(SP), R4
	0x0034 00052 	     	NOP
	0x0034 00052 	     	NOP
	0x0034 00052 	     	NOP
	0x0034 00052 	L1303	MOVD	R2, internal/abi.a-32(SP)
	0x0038 00056 	L1303	MOVD	R3, internal/abi.a-24(SP)
	0x003c 00060 	L0179	MOVD	internal/abi.a-32(SP), R3
	0x0040 00064 	     	NOP
	0x0040 00064 	     	NOP
	0x0040 00064 	L2719	CMP	$0, R4
	0x0044 00068 	L0510	MOVD	$go:itab.*reflect.rtype,reflect.Type(SB), R5
	0x004c 00076 	L0510	CSEL	NE, R5, ZR, R0
	0x0050 00080 	L2719	CMP	$0, R3
	0x0054 00084 	L0510	CSEL	NE, R5, ZR, R5
	0x0058 00088 	L0510	CMP	R5, R0
	0x005c 00092 	L2719	BEQ	104
	0x0060 00096 	L2719	MOVD	$1, R3
	0x0064 00100 	L2719	JMP	128
	0x0068 00104 	L2719	CMP	$0, R4
	0x006c 00108 	L0510	CSEL	NE, R4, ZR, R1
	0x0070 00112 	L2719	CMP	$0, R3
	0x0074 00116 	L0510	CSEL	NE, R3, ZR, R2
	0x0078 00120 	L0510	PCDATA	$1, $1
	0x0078 00120 	L0510	CALL	runtime.ifaceeq(SB)
	0x007c 00124 	L0510	EOR	$1, R0, R3
	0x0080 00128 	L0510	MOVD	R3, R0
	0x0084 00132 	L0510	MOVD	-8(RSP), R29
	0x0088 00136 	L0510	MOVD.P	80(RSP), R30
	0x008c 00140 	L0510	RET	(R30)
	0x0090 00144 	L0510	NOP
	0x0090 00144 	L0505	PCDATA	$1, $-1
	0x0090 00144 	L0505	PCDATA	$0, $-2
	0x0090 00144 	L0505	STP	(R0, R1), 8(RSP)
	0x0094 00148 	L0505	STP	(R2, R3), 24(RSP)
	0x0098 00152 	L0505	MOVD	R30, R3
	0x009c 00156 	L0505	CALL	runtime.morestack_noctxt(SB)
	0x00a0 00160 	L0505	PCDATA	$0, $-1
	0x00a0 00160 	L0505	LDP	8(RSP), (R0, R1)
	0x00a4 00164 	L0505	LDP	24(RSP), (R2, R3)
	0x00a8 00168 	L0505	JMP	0

Is there any reason it couldn't be recognized as a special case and compiled as if written using unsafe Go code like this:

func sameType(x, y any) bool {
	type eface struct {
		t, v unsafe.Pointer
	}
	return ((*eface)(unsafe.Pointer(&x))).t == ((*eface)(unsafe.Pointer(&y))).t
}

which boils down to a pair of loads and a comparison:

golang.org/x/tools/gopls/internal/golang.sameType STEXT size=48 args=0x20 locals=0x0 funcid=0x0 align=0x0 leaf
	0x0000 00000 	L0505	TEXT	golang.org/x/tools/gopls/internal/golang.sameType(SB), LEAF|NOFRAME|ABIInternal, $0-32
	0x0000 00000 	L0505	FUNCDATA	$0, gclocals·emgMVsd5mo9g0HGy8lQYQA==(SB)
	0x0000 00000 	L0505	FUNCDATA	$1, gclocals·acF1O9X4FQHZUTLQivBEZA==(SB)
	0x0000 00000 	L0505	FUNCDATA	$2, golang.org/x/tools/gopls/internal/golang.sameType.stkobj(SB)
	0x0000 00000 	L0505	FUNCDATA	$5, golang.org/x/tools/gopls/internal/golang.sameType.arginfo1(SB)
	0x0000 00000 	L0505	MOVD	R0, golang.org/x/tools/gopls/internal/golang.x(FP)
	0x0004 00004 	L0505	MOVD	R1, golang.org/x/tools/gopls/internal/golang.x+8(FP)
	0x0008 00008 	L0505	MOVD	R2, golang.org/x/tools/gopls/internal/golang.y+16(FP)
	0x000c 00012 	L0505	MOVD	R3, golang.org/x/tools/gopls/internal/golang.y+24(FP)
	0x0010 00016 	L0509	MOVD	golang.org/x/tools/gopls/internal/golang.x(FP), R1
	0x0014 00020 	L0509	MOVD	golang.org/x/tools/gopls/internal/golang.y+16(FP), R2
	0x0018 00024 	L0509	CMP	R1, R2
	0x001c 00028 	L0509	CSET	EQ, R0
	0x0020 00032 	L0509	RET	(R30)
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 9, 2024
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 9, 2024
@cagedmantis cagedmantis added this to the Backlog milestone Dec 9, 2024
@griesemer griesemer modified the milestones: Backlog, Go1.25 Dec 11, 2024
@griesemer griesemer assigned griesemer and randall77 and unassigned randall77 and griesemer Dec 11, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/635435 mentions this issue: cmd/compile: avoid ifaceeq call if we know the interface is direct

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. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

6 participants