cmd/compile: no instruction marked with is_stmt in debug_line for statement calling generic function, inside a generic function #49628

aarzilli · 2021-11-17T10:44:45Z

go version devel go1.18-1d004fa201 Wed Nov 17 04:55:12 2021 +0000 linux/amd64

package main

import "fmt"

func F[T any](n T) {
	fmt.Printf("called\n")
}

func G[T any](n T) {
	F(n)
	fmt.Printf("after\n")
}

func main() {
	G(3)
}

The instantiation of main.G has no instruction marked is_stmt for the call to main.F.

The text was updated successfully, but these errors were encountered:

aarzilli · 2021-11-17T10:45:02Z

cc @dr2chase @danscales

danscales · 2021-11-17T21:09:03Z

@dr2chase Thanks for taking a look. I don't know much about the is_stmt markers, and it looks likes WithIsStmt() is mostly used in SSA and inlining, so I'm unsure what I might need to fix with the stenciling/instantiation code.

Let me know if you want to talk about this - maybe I can take over once if you figure out there is something I should be doing in the instantiation code or the code that initially creates the nodes for generic functions (but that is the same code that creates nodes for non-generic functions).

dr2chase · 2021-11-19T21:03:49Z

I used go build -gcflags=-d=ssa/all/dump='G[go.shape.int_0]' aa.go to get a better description of what was going on (it is like GOSSAFUNC output, but phase-by-phase into files, with the inlining locations fully expanded so we can see what's going on). Any line marked with a "+" has IsStmt set. I don't immediately see the problem (I see an instruction with line "+10") but I could be overlooking something.

# /Users/drchase/work/tmp/b49628/aa.go:9
       	00000 (9) 	TEXT	"".G[go.shape.int_0](SB), DUPOK|ABIInternal
       	00001 (9) 	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
       	00002 (9) 	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
       	00003 (9) 	FUNCDATA	$5, "".G[go.shape.int_0].arginfo1(SB)
       	00004 (9) 	FUNCDATA	$6, "".G[go.shape.int_0].argliveinfo(SB)
 b4    	00005 (9) 	PCDATA	$3, $1
 v10   	00006 (+10) 	TESTB	AX, (AX)
 v15   	00007 (?) 	NOP
# /Users/drchase/work/tmp/b49628/aa.go:10
# /Users/drchase/work/tmp/b49628/aa.go:6
 v20   	00008 (+6) 	XCHGL	AX, AX
# /Users/drchase/work/tmp/b49628/aa.go:10
# /Users/drchase/work/tmp/b49628/aa.go:6
# $GOROOT/src/fmt/print.go:213
 v23   	00009 (+213) 	MOVQ	os.Stdout(SB), BX
 v33   	00010 (-213) 	LEAQ	go.itab.*os.File,io.Writer(SB), AX
 v22   	00011 (-213) 	LEAQ	go.string."called\n"(SB), CX
 v34   	00012 (-213) 	MOVL	$7, DI
 v36   	00013 (-213) 	XORL	SI, SI
 v24   	00014 (-213) 	XORL	R8, R8
 v30   	00015 (-213) 	MOVQ	R8, R9
 v25   	00016 (-213) 	PCDATA	$1, $0
 v25   	00017 (-213) 	CALL	fmt.Fprintf(SB)
 v32   	00018 (?) 	NOP
# /Users/drchase/work/tmp/b49628/aa.go:11
# $GOROOT/src/fmt/print.go:213
 v35   	00019 (-213) 	MOVQ	os.Stdout(SB), BX
 v17   	00020 (-213) 	LEAQ	go.itab.*os.File,io.Writer(SB), AX
 v16   	00021 (-213) 	LEAQ	go.string."after\n"(SB), CX
 v28   	00022 (-213) 	MOVL	$6, DI
 v41   	00023 (-213) 	XORL	SI, SI
 v19   	00024 (-213) 	XORL	R8, R8
 v18   	00025 (-213) 	MOVQ	R8, R9
 v37   	00026 (-213) 	CALL	fmt.Fprintf(SB)
# /Users/drchase/work/tmp/b49628/aa.go:11
 b4    	00027 (+11) 	RET
       	00028 (?) 	END

danscales · 2021-11-19T21:30:22Z

Thanks, David. Note that the output @dr2chase is showing is from the G[go.shape.int_0]_52__genssa.dump file written to the file system.

Here's same output, but for aa2.go in which we convert the generic functions to non-generic functions where T is int:

package main

import "fmt"

func F(n int) {
        fmt.Printf("called\n")
}

func G(n int) {
        F(n)
        fmt.Printf("after\n")
}

func main() {
        G(3)
}

The output is now:

# aa2.go:9
       	00000 (9) 	TEXT	"".G(SB), ABIInternal
       	00001 (9) 	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
       	00002 (9) 	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
       	00003 (9) 	FUNCDATA	$5, "".G.arginfo1(SB)
       	00004 (9) 	FUNCDATA	$6, "".G.argliveinfo(SB)
 b4    	00005 (9) 	PCDATA	$3, $1
 v6    	00006 (?) 	NOP
# aa2.go:10
# aa2.go:6
 v11   	00007 (+6) 	XCHGL	AX, AX
# aa2.go:10
 v14   	00008 (+10) 	MOVQ	os.Stdout(SB), BX
# aa2.go:10
# aa2.go:6
# $GOROOT/src/fmt/print.go:213
 v24   	00009 (+213) 	LEAQ	go.itab.*os.File,io.Writer(SB), AX
 v13   	00010 (-213) 	LEAQ	go.string."called\n"(SB), CX
 v25   	00011 (-213) 	MOVL	$7, DI
 v27   	00012 (-213) 	XORL	SI, SI
 v21   	00013 (-213) 	XORL	R8, R8
 v15   	00014 (-213) 	MOVQ	R8, R9
 v16   	00015 (-213) 	PCDATA	$1, $0
 v16   	00016 (-213) 	CALL	fmt.Fprintf(SB)
# aa2.go:11
 v23   	00017 (+11) 	XCHGL	AX, AX
# aa2.go:11
# $GOROOT/src/fmt/print.go:213
 v26   	00018 (-213) 	MOVQ	os.Stdout(SB), BX
 v8    	00019 (-213) 	LEAQ	go.itab.*os.File,io.Writer(SB), AX
 v7    	00020 (-213) 	LEAQ	go.string."after\n"(SB), CX
 v19   	00021 (-213) 	MOVL	$6, DI
 v32   	00022 (-213) 	XORL	SI, SI
 v10   	00023 (-213) 	XORL	R8, R8
 v9    	00024 (-213) 	MOVQ	R8, R9
 v28   	00025 (-213) 	CALL	fmt.Fprintf(SB)
# aa2.go:12
 b4    	00026 (12) 	RET
       	00027 (?) 	END

Both have the same assembly statements marked with '+' (though aa.go has an extra assembly statement 'TESTB AX, (AX)' that is marked +10). The main difference I see is that the 'MOVQ os.Stdout(SB), BX' statement is marked (+10) rather than (+213).

@aarzilli Can you point out more of what the problem is that you are seeing? Is it something related to the difference for the 'MOVQ os.Stdout(SB), BX' statement (even though in both cases it is marked as IsStmt)?

aarzilli · 2021-11-20T07:52:03Z

Interesting. It looks like it only happens with optimizations disabled.

go build -x -gcflags='-N -l -d=ssa/all/dump=G[go.shape.int_0]' test.go

       	00000 (9) 	TEXT	"".G[go.shape.int_0](SB), DUPOK|ABIInternal
       	00001 (9) 	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
       	00002 (9) 	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
       	00003 (9) 	FUNCDATA	$5, "".G[go.shape.int_0].arginfo1(SB)
 v32   	00004 (+9) 	PCDATA	$0, $-2
 v32   	00005 (+9) 	MOVQ	AX, ""..dict(SP)
 v22   	00006 (-9) 	MOVQ	BX, "".n+8(SP)
 v13   	00007 (-10) 	PCDATA	$0, $-1
 v13   	00008 (-10) 	TESTB	AX, (AX)
 v17   	00009 (-10) 	MOVQ	16(AX), AX
 v21   	00010 (-10) 	MOVQ	AX, ""..autotmp_2-8(SP)
 v23   	00011 (-10) 	MOVQ	"".n+8(SP), BX
 v24   	00012 (-10) 	PCDATA	$1, $0
 v24   	00013 (-10) 	CALL	"".F[go.shape.int_0](SB)
 v19   	00014 (+11) 	LEAQ	go.string."after\n"(SB), AX
 v16   	00015 (-11) 	MOVL	$6, BX
 v5    	00016 (-11) 	XORL	CX, CX
 v4    	00017 (-11) 	XORL	DI, DI
 v27   	00018 (-11) 	MOVQ	DI, SI
 v28   	00019 (+11) 	CALL	fmt.Printf(SB)
 b1    	00020 (-11) 	RET
       	00021 (?) 	END

(the step before the last one does have an instruction marked with +)

danscales · 2021-11-22T17:13:07Z

OK, so I think that you are pointing out that line 10 does not have any associated assembly instruction that is marked with "+10".

If I compare the dump output for test.go and the aa2.go about that I showed above where we substitute in 'int' for T everywhere, I see that up to the last SSA pass, both test.go and aa2.go have values that are marked with "+10". But in the test.go (generic case), the value that has the +10 is the conversion to unsafe pointer:

(+10) v11 = Convert <unsafe.Pointer> v33 v22 : AX

This is the conversion of the dictionary argument to unsafe pointer before then converting to a *[3]uintptr that we can then index into. When we convert to assembly, this unsafe pointer conversion is just dropped, and the +10 is not transferred to any other statement (it appears).

I'm not sure what the most consistent fix is. The most obvious fix is to find where the unsafe pointer conversion is essentially dropped (as a no-op), and make sure the isStmt attributed is passed to the next assembly statement.

On the other hand, not sure if we should somehow be attributing all the dictionary loading to the previous statement (line 9) so that the beginning of line 10 is more directly already focused on the call. This seems less likely to me, but the load of the other argument is already attributed to line 9 (the load of n into the BX register).

@randall77 @dr2chase

dr2chase · 2021-11-23T16:25:21Z

Pretty sure there's a bad interaction with the LoweredNilCheck -- we don't like to float statement markers past those, because setting breakpoints AFTER the nil check gives bad user experience in the case that user is trying to debug a nil panic.

dr2chase · 2021-11-23T16:45:25Z

I have what looks like a fix, curious what it does to other line number placement.
Ought to be strictly better.

gopherbot · 2021-11-23T18:33:20Z

Change https://golang.org/cl/366694 mentions this issue: cmd/compile: try to preserve IsStmt marks from OpConvert

dr2chase self-assigned this Nov 17, 2021

dr2chase added the NeedsInvestigation label Nov 17, 2021

gopherbot closed this as completed in 61011de Dec 8, 2021

rsc unassigned dr2chase Jun 23, 2022

aarzilli mentioned this issue Jan 4, 2023

breakpoint not hit and missing stack frames from runtime go-delve/delve#3233

Closed

golang locked and limited conversation to collaborators Jun 23, 2023

gopherbot added the FrozenDueToAge label Jun 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: no instruction marked with is_stmt in debug_line for statement calling generic function, inside a generic function #49628

cmd/compile: no instruction marked with is_stmt in debug_line for statement calling generic function, inside a generic function #49628

aarzilli commented Nov 17, 2021

aarzilli commented Nov 17, 2021

danscales commented Nov 17, 2021

dr2chase commented Nov 19, 2021

danscales commented Nov 19, 2021 •

edited

Loading

aarzilli commented Nov 20, 2021

danscales commented Nov 22, 2021

dr2chase commented Nov 23, 2021

dr2chase commented Nov 23, 2021

gopherbot commented Nov 23, 2021

cmd/compile: no instruction marked with is_stmt in debug_line for statement calling generic function, inside a generic function #49628

cmd/compile: no instruction marked with is_stmt in debug_line for statement calling generic function, inside a generic function #49628

Comments

aarzilli commented Nov 17, 2021

aarzilli commented Nov 17, 2021

danscales commented Nov 17, 2021

dr2chase commented Nov 19, 2021

danscales commented Nov 19, 2021 • edited Loading

aarzilli commented Nov 20, 2021

danscales commented Nov 22, 2021

dr2chase commented Nov 23, 2021

dr2chase commented Nov 23, 2021

gopherbot commented Nov 23, 2021

danscales commented Nov 19, 2021 •

edited

Loading