Commit 6817210e authored by Josh Bleecher Snyder's avatar Josh Bleecher Snyder
Browse files

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: default avatarKeith Randall <khr@golang.org>
parent 24fa159a
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment