]> Cypherpunks repositories - gostls13.git/commit
cmd/compile: mark amd64 HMUL ops as not commutative
authorJosh Bleecher Snyder <josharian@gmail.com>
Tue, 7 Jan 2020 17:27:18 +0000 (09:27 -0800)
committerJosh Bleecher Snyder <josharian@gmail.com>
Thu, 20 Feb 2020 14:58:45 +0000 (14:58 +0000)
commit6817210edf5b4c54ddb0948fff0ed491456b470c
treeaffb12d5bf234de7efa7bbcf303e3d488e562154
parent24fa159a2e5cba1c0038af7ae55802eb58fcb591
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 <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
src/cmd/compile/internal/ssa/gen/AMD64Ops.go
src/cmd/compile/internal/ssa/opGen.go