Skip to content

Commit 6817210

Browse files
committed
cmd/compile: mark amd64 HMUL ops as not commutative
HMUL is commutative. However, it has asymmetric register requirements. There are existing rewrite rules to place arguments in preferable slots. Due to a bug, the existing rulegen commutativity engine doesn't generate the commuted form of the HMUL rules. The commuted form of those rewrite rules cause infinite loops. In order to fix the rulegen commutativity bug, we need to choose between eliminating those rewrite rules and marking HMUL ops as not commutative. This change chooses the latter, since doing so yields better optimization results on std+cmd. Removing the rewrite rules yields only text size regressions: file before after Δ % runtime.s 477257 477269 +12 +0.003% time.s 83552 83612 +60 +0.072% encoding/asn1.s 57378 57382 +4 +0.007% cmd/go/internal/modfetch/codehost.s 89822 89829 +7 +0.008% cmd/internal/test2json.s 9459 9466 +7 +0.074% cmd/go/internal/test.s 57665 57678 +13 +0.023% Marking HMUL as not commutative actually yields (mostly) improvements: file before after Δ % runtime.s 477257 477247 -10 -0.002% math.s 35985 35992 +7 +0.019% strconv.s 53486 53462 -24 -0.045% syscall.s 82483 82446 -37 -0.045% time.s 83552 83561 +9 +0.011% os.s 52691 52684 -7 -0.013% archive/zip.s 42285 42272 -13 -0.031% encoding/asn1.s 57378 57329 -49 -0.085% encoding/base64.s 12156 12094 -62 -0.510% net.s 296286 296276 -10 -0.003% encoding/base32.s 9720 9658 -62 -0.638% net/http.s 560931 560907 -24 -0.004% net/smtp.s 14421 14411 -10 -0.069% cmd/vendor/golang.org/x/sys/unix.s 74307 74266 -41 -0.055% The regressions are minor, and are in functions math.cbrt, time.Time.String, and time.Date. Change-Id: I9f6d9ee71654e5b70381cac77b0ac26011f4ea12 Reviewed-on: https://go-review.googlesource.com/c/go/+/213701 Run-TryBot: Josh Bleecher Snyder <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 24fa159 commit 6817210

File tree

2 files changed

+7
-8
lines changed

2 files changed

+7
-8
lines changed

src/cmd/compile/internal/ssa/gen/AMD64Ops.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,13 @@ func init() {
220220
{name: "MULLU", argLength: 2, reg: regInfo{inputs: []regMask{ax, gpsp}, outputs: []regMask{ax, 0}, clobbers: dx}, typ: "(UInt32,Flags)", asm: "MULL", commutative: true, clobberFlags: true}, // Let x = arg0*arg1 (full 32x32->64 unsigned multiply). Returns uint32(x), and flags set to overflow if uint32(x) != x.
221221
{name: "MULQU", argLength: 2, reg: regInfo{inputs: []regMask{ax, gpsp}, outputs: []regMask{ax, 0}, clobbers: dx}, typ: "(UInt64,Flags)", asm: "MULQ", commutative: true, clobberFlags: true}, // Let x = arg0*arg1 (full 64x64->128 unsigned multiply). Returns uint64(x), and flags set to overflow if uint64(x) != x.
222222

223-
{name: "HMULQ", argLength: 2, reg: gp21hmul, commutative: true, asm: "IMULQ", clobberFlags: true}, // (arg0 * arg1) >> width
224-
{name: "HMULL", argLength: 2, reg: gp21hmul, commutative: true, asm: "IMULL", clobberFlags: true}, // (arg0 * arg1) >> width
225-
{name: "HMULQU", argLength: 2, reg: gp21hmul, commutative: true, asm: "MULQ", clobberFlags: true}, // (arg0 * arg1) >> width
226-
{name: "HMULLU", argLength: 2, reg: gp21hmul, commutative: true, asm: "MULL", clobberFlags: true}, // (arg0 * arg1) >> width
223+
// HMULx[U] are intentionally not marked as commutative, even though they are.
224+
// This is because they have asymmetric register requirements.
225+
// There are rewrite rules to try to place arguments in preferable slots.
226+
{name: "HMULQ", argLength: 2, reg: gp21hmul, asm: "IMULQ", clobberFlags: true}, // (arg0 * arg1) >> width
227+
{name: "HMULL", argLength: 2, reg: gp21hmul, asm: "IMULL", clobberFlags: true}, // (arg0 * arg1) >> width
228+
{name: "HMULQU", argLength: 2, reg: gp21hmul, asm: "MULQ", clobberFlags: true}, // (arg0 * arg1) >> width
229+
{name: "HMULLU", argLength: 2, reg: gp21hmul, asm: "MULL", clobberFlags: true}, // (arg0 * arg1) >> width
227230

228231
{name: "AVGQU", argLength: 2, reg: gp21, commutative: true, resultInArg0: true, clobberFlags: true}, // (arg0 + arg1) / 2 as unsigned, all 64 result bits
229232

src/cmd/compile/internal/ssa/opGen.go

Lines changed: 0 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)