cmd/compile: bad line number attached to LEA instruction

[aarzilli]

go version devel +eaf6036 Fri Nov 3 01:55:16 2017 +0000 linux/amd64

This function gets compiled into this (with -N -l):

check.go:17	0x589cf0	MOVQ FS:0xfffffff8, CX
check.go:17	0x589cf9	LEAQ 0xfffffbb0(SP), AX
check.go:17	0x589d01	 CMPQ 0x10(CX), AX
check.go:17	0x589d05	JBE $0x58ab52
check.go:17	0x589d0b	 SUBQ $0x4d0, SP
check.go:17	0x589d12	 MOVQ BP, 0x4c8(SP)
check.go:17	0x589d1a	 LEAQ 0x4c8(SP), BP
check.go:17	0x589d22	 XORPS X0, X0
check.go:17	0x589d25	 LEAQ 0x348(SP), DI
check.go:17	0x589d2d	 MOVQ BP, -0x10(SP)
check.go:17	0x589d32	 LEAQ -0x10(SP), BP
check.go:17	0x589d37	 CALL $runtime.duffzero+247(SB)
check.go:17	0x589d3c	 MOVQ 0(BP), BP
check.go:17	0x589d40	 MOVQ $0x0, 0x4f0(SP)
check.go:18	0x589d4c	 MOVQ 0x4d8(SP), AX
check.go:18	0x589d54	 TESTB AL, 0(AX)
check.go:18	0x589d56	 MOVQ 0x40(AX), AX
check.go:67	0x589d5a	 LEAQ 0x8(SP), CX
check.go:18	0x589d5f	TESTQ AX, AX
(cut)

note the line number for 0x589d5a.

This also happens with 1.9.1, not a regression.

cc @heschik @dr2chase

[dr2chase]

We have a theory about how to solve this, which is basically to stop splattering line numbers on everything unless we are really really sure a line number would be a good idea. I am happy to look into this for 1.10, I verified that it reproduces with the debugger changes I currently have pending.

[mdempsky]

Looking at GOSSAFUNC output, I think line 67 is actually accurate here for the LEAQ. The fact that it shows up here is a symptom of poor optimization (which I think is reasonably expected under -N -l).

What's happening is the isJump(fn, inst) call at line 67 needs to emit an OpMove instruction to copy inst to the parameter slot before the OpStaticCall, which in turns needs an instruction to compute the stack address. Even though this offset is pretty easy to compute, and probably could be folded into OpMove lowering, we need an LEAQ instruction to compute it.

Further, for whatever reasons, the LEAQ instruction bubbles to the top of the function, where you noticed it. (And embarrassingly ends up needing to be spilled to a stack variable, even in optimized builds.)

[aarzilli]

Further, for whatever reasons, the LEAQ instruction bubbles to the top of the function

I looked into this yesterday, it happens because buildssa sticks pretty much every constant on the entry block of the function. So, yes, that's the correct src.Pos for that instruction but the control flow was altered significantly which results in a very poor debugging experience.

IMHO altering the order of execution does count as optimization, and shouldn't happen in a -N build. If it's decided that it really doesn't matter where that instruction is executed, because it's a constant, then it should have its Pos cleared.

[dr2chase]

If it's decided that it really doesn't matter where that instruction is executed, because it's a constant, then it should have its Pos cleared.

I can try this experiment pretty quickly, let me see how it goes. There's a few hours left to make the 1.10 beta.

[gopherbot]

Change https://golang.org/cl/81215 mentions this issue: cmd/compile: use src.NoXPos for entry-block constants

[mdempsky]

and probably could be folded into OpMove lowering

The issue here is inst is 728 bytes large, so OpMove is lowered to DUFFCOPY. If it was smaller, we would lower it to Load+Store pairs, which could fold the LEAQ into the MOV addressing.

It seems like the problem is OffPtr(N, SP) isn't being recognized as rematerializeable by the register allocator, because it's lowered to ADDQconst(N, SP), and ADDQconst isn't a rematerializeable operation.

IMHO altering the order of execution does count as optimization, and shouldn't happen in a -N build.

I'm sympathetic to the fact that this is a poor debugging experience, and understanding the 67 line took a fair amount of compiler knowledge, but I think this is a fairly strong interpretation of -N's intent.

[aarzilli]

I'm sympathetic to the fact that this is a poor debugging experience, and understanding the 67 line took a fair amount of compiler, but I think this is a fairly strong interpretation of -N's intent.

Yes, it's strong but I think it's reasonable. We're getting dangerously close to Theseus paradox, but I think an average user would expect a non-optimized build to not alter the control flow of the program for example wikipedia says:

While writing an application, a programmer will recompile and test often, and so compilation must be fast. This is one reason most optimizations are deliberately avoided during the test/debugging phase. Also, program code is usually "stepped through" (see Program animation) using a symbolic debugger, and optimizing transformations, particularly those that reorder code, can make it difficult to relate the output code with the line numbers in the original source code. This can confuse both the debugging tools and the programmers using them.