cmd/compile: rematerializable ops must not clobber flags

[mundaym]

While debugging #21048 I noticed that there was a conditional jump immediately after a MOVDaddr instruction. This shouldn't happen because MOVDaddr clobbers flags on s390x.

Digging deeper it looks to me like flagalloc is silently failing in runtime.gcDrain. The problem is reproducible on darwin/amd64.

If you do:

GOSSAFUNC=gcDrain go install -a runtime

And look at the generated ssa.html after the regalloc phase you'll see this:

b3: ← b1
    v17 = LoweredGetG <*g> v1 : AX
    v20 = MOVQload <*m> [48] v17 v1 : AX
    v23 = MOVQload <*g> [152] v20 v1 : AX
    v122 = StoreReg <*g> v23 : gp[*g]
    v142 = LoadReg <*gcWork> v6 : CX
    v41 = MOVQload <int64> [24] v142 v1 : DX
    v46 = MOVLload <uint32> {"".work} [136] v3 v1 : BX
    v49 = MOVLload <uint32> {"".work} [140] v3 v1 : SI
    v187 = LoadReg <gcDrainFlags> v7 : DI
    v18 = BTLconst <flags> [0] v187    <===== v18 SET
    v356 = BTLconst <flags> [2] v187   <===== v18 CLOBBERED
    v234 = BTLconst <flags> [3] v187
    v197 = CMPL <flags> v46 v49
ULT v197 → b4 b48

b4: ← b3
    v204 = StoreReg <int64> v41 : initScanWork[int64]
    v100 = BTLconst <flags> [0] v187
Plain → b6

b6: ← b4 b8
    v365 = Phi <mem> v1 v366
ULT v18 → b10 b40                      <==== v18 USED

v18 is clobbered immediately. v356 and v234 are generated but immediately clobbered. All these BTL instructions make it into the final assembly.

[mundaym]

/cc @randall77 @dr2chase @cherrymui

[mundaym]

Ah, looking through the flagalloc code I see this is the expected behavior because the flags are regenerated as necessary, but the values aren't hooked up.

My problem is actually the regalloc pass inserting MOVDaddr ops after the flagalloc pass. I guess it assumes the op doesn't clobber flags. Probably the right fix is to try and make MOVDaddr not clobber flags on s390x somehow (currently it can insert an ADD with 32-bit immediate in shared mode, hence the clobber flags mark).

I suspect that can wait until 1.10. It's probably also worth trying to get rid of the redundant instructions, not sure if there is already an issue for that?

[cherrymui]

Yeah, the register allocator may insert MOVDaddr because it is "rematerializable". But if it may clobber flags, it is not really rematerializable. Marking MOVDaddr not rematerializable probably makes liveness unhappy, though.

Is there an instruction that does the address calculation without clobbering flags, like LEA on 386? I tried to change ADD $n, Rx to MOVD $n(Rx), Rx in rewriteToUseGot, but it didn't work (seg fault). Apparently I missed something...

[mundaym]

Yeah, we have LAY, but it has a couple of limitations so it's not quite a straight swap. Firstly it can't take R0 as in input, but we could easily workaround that by removing R0 from the list of valid MOVDaddr targets. The second issue is that the offset is limited to a 20-bit signed integer which means we need to use a temp register when the offset is larger.

[gopherbot]

Change https://golang.org/cl/63030 mentions this issue: cmd/compile: stop MOVDaddr clobbering flags on s390x