cmd/internal/obj/arm: fix/rationalize checkpool distance check

When deciding whether to flush the constant pool, the distance check
in checkpool can fail to account for padding inserted before the next
instruction by nacl.

For example, see this failure:
https://go-review.googlesource.com/c/go/+/109350/2#message-07085b591227824bb1d646a7192cbfa7e0b97066
Here, the pool should be flushed before a CALL instruction, but
checkpool only considers the CALL instruction to be 4 bytes and
doesn't account for the 8 extra bytes of alignment padding added
before it by asmoutnacl. As a result, it flushes the pool after the
CALL instruction, which is 4 bytes too late.

Furthermore, there's no explanation for the rather convoluted
expression used to decide if we need to emit the constant pool.

This CL modifies checkpool to take the PC following the tentative
instruction as an argument. The caller knows this already and this way
checkpool doesn't have to guess (and get it wrong in the presence of
padding). In the process, it rewrites the test to be structured and
commented.

Change-Id: I32a3d50ffb5a94d42be943e9bcd49036c7e9b95c
Reviewed-on: https://go-review.googlesource.com/110017
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/internal/obj/arm/asm5.go b/src/cmd/internal/obj/arm/asm5.go
index a88ae74d311d5efc06fb7f37a55dd5c8fcf430f3..96f6b90e8d5eb2e4434c85bd564366f8c1b061aa 100644 (file)
--- a/src/cmd/internal/obj/arm/asm5.go
+++ b/src/cmd/internal/obj/arm/asm5.go
@@ -668,12 +668,11 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
 
        op = p
        p = p.Link
-       var i int
        var m int
        var o *Optab
        for ; p != nil || c.blitrl != nil; op, p = p, p.Link {
                if p == nil {
-                       if c.checkpool(op, 0) {
+                       if c.checkpool(op, pc) {
                                p = op
                                continue
                        }
@@ -700,8 +699,12 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
 
                // must check literal pool here in case p generates many instructions
                if c.blitrl != nil {
-                       i = m
-                       if c.checkpool(op, i) {
+                       // Emit the constant pool just before p if p
+                       // would push us over the immediate size limit.
+                       if c.checkpool(op, pc+int32(m)) {
+                               // Back up to the instruction just
+                               // before the pool and continue with
+                               // the first instruction of the pool.
                                p = op
                                continue
                        }
@@ -872,7 +875,7 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
                        pc += 4
                }
 
-               for i = 0; i < m/4; i++ {
+               for i := 0; i < m/4; i++ {
                        v = int(out[i])
                        bp[0] = byte(v)
                        bp = bp[1:]
@@ -888,14 +891,26 @@ func span5(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
        }
 }
 
-/*
- * when the first reference to the literal pool threatens
- * to go out of range of a 12-bit PC-relative offset,
- * drop the pool now, and branch round it.
- * this happens only in extended basic blocks that exceed 4k.
- */
-func (c *ctxt5) checkpool(p *obj.Prog, sz int) bool {
-       if c.pool.size >= 0xff0 || immaddr(int32((p.Pc+int64(sz)+4)+4+int64(12+c.pool.size)-int64(c.pool.start+8))) == 0 {
+// checkpool flushes the literal pool when the first reference to
+// it threatens to go out of range of a 12-bit PC-relative offset.
+//
+// nextpc is the tentative next PC at which the pool could be emitted.
+// checkpool should be called *before* emitting the instruction that
+// would cause the PC to reach nextpc.
+// If nextpc is too far from the first pool reference, checkpool will
+// flush the pool immediately after p.
+// The caller should resume processing a p.Link.
+func (c *ctxt5) checkpool(p *obj.Prog, nextpc int32) bool {
+       poolLast := nextpc
+       poolLast += 4                      // the AB instruction to jump around the pool
+       poolLast += 12                     // the maximum nacl alignment padding for ADATABUNDLE
+       poolLast += int32(c.pool.size) - 4 // the offset of the last pool entry
+
+       refPC := int32(c.pool.start) // PC of the first pool reference
+
+       v := poolLast - refPC - 8 // 12-bit PC-relative offset (see omvl)
+
+       if c.pool.size >= 0xff0 || immaddr(v) == 0 {
                return c.flushpool(p, 1, 0)
        } else if p.Link == nil {
                return c.flushpool(p, 2, 0)
@@ -1016,6 +1031,7 @@ func (c *ctxt5) addpool(p *obj.Prog, a *obj.Addr) {
        c.elitrl = q
        c.pool.size += 4
 
+       // Store the link to the pool entry in Pcond.
        p.Pcond = q
 }