I did a little archeology on this. The block that the checker is complaining about is:

  b11: <- b14 b10
    v48 = RegKill <void> : R9 (next[int64])
    v82 = Phi <int64> v41 v56 : R9 (next[int64])
    v47 = RegKill <void> : DX (x[int64])
    v50 = SARQconst <int64> [1] v83 : DX (x[int64])
    Plain -> b9

which clearly violates the "no non-phi after phi rule", but in fact this specific testpoint wasn't added until October, meaning that the locations lists work predates the test (the location lists code was written back in June(ish), then the dev.debug branch was merged into master in August).

I'm not familiar enough with the SSA code to know what the right fix is. One thing to do would just be to relax the checking rules to ignore RegKills, since it is pretty clear that they don't correspond to any sort of real value definition.

David, maybe you could weigh in on this. @dr2chase

Thanks. This is working as I intended at the time, and the test could probably be updated to ignore the RegKill ops. Phis put values in registers like anything else, and if one clobbers something that was already in the register before that, there should be a RegKill before it. That was a change in the invariants of the SSA form, but it didn't cause much trouble.

In this particular case, since the set of names that's being set on the registers by the Phis match the ones being removed by the RegKills, they're pointless. But I'm pretty sure there are cases where the set of names differ, and then they are necessary.

Change https://golang.org/cl/77610 mentions this issue: cmd/compile: ignore RegKill ops for non-phi after phi check

Beyond the SSA sanity checker (which should be fixed with 77610) there are also a couple of GDB test failures:

--- FAIL: TestGdbPython (2.16s)
	runtime-gdb_test.go:55: gdb version 7.9
	runtime-gdb_test.go:210: print strvar failed: $2 = <optimized out>
--- FAIL: TestGdbPythonCgo (2.54s)
	runtime-gdb_test.go:55: gdb version 7.9
	runtime-gdb_test.go:210: print strvar failed: $2 = <optimized out>

It looks like these two above will go away once my DWARF inlining patch is submitted. After that, there are a set of failures of the following form:

--- FAIL: TestAssembly (2.78s)
    --- FAIL: TestAssembly/platform (0.00s)
        --- FAIL: TestAssembly/platform/linux/mips (0.12s)
        	asm_test.go:213: error running cmd: exit status 2
        		stdout:
        		stderr:
        		# math/bits
        		compile: location lists requested but register mapping not available on mips

These are due to this line in cmd/compile/internal/gc/main.go:

	if Ctxt.Flag_locationlists && len(Ctxt.Arch.DWARFRegisters) == 0 {
		log.Fatalf("location lists requested but register mapping not available on %v", Ctxt.Arch.Name)
	}

meaning someone needs to create fill in the DWARF registers for the various non-x86 archs.

Huh. Interesting that the inlining patch would fix the GDB test. There must be some strange stuff going on with the name table.

The tests mentioned above all seem to pass when run with -gcflags=-dwarflocationlists. Is this still a problem?

Thanks Ian-- yes, I agree this can be closed out.