aarch64: Fix PR96998 and restore code quality in combine
authorAlex Coplan <alex.coplan@arm.com>
Fri, 30 Oct 2020 09:21:31 +0000 (09:21 +0000)
committerAlex Coplan <alex.coplan@arm.com>
Fri, 30 Oct 2020 09:21:31 +0000 (09:21 +0000)
commit7de23b8c536397117bbea04a722fa1b86564dd7c
tree482047823141481f576088ffc9a8b477768bcb6f
parent17e8da49bfe6a91d13d30888cb9cea0ee255b945
aarch64: Fix PR96998 and restore code quality in combine

This change fixes a bug in the AArch64 backend. Currently, we accept an
odd sign_extract representation of addresses, but don't accept that same
odd form of address as an LEA.

This is the cause of PR96998. In the testcase given in the PR, combine
produces:

(insn 9 8 10 3 (set (mem:SI (plus:DI (sign_extract:DI (mult:DI (subreg:DI (reg/v:SI 92 [ g ]) 0)
                        (const_int 4 [0x4]))
                    (const_int 34 [0x22])
                    (const_int 0 [0]))
                (reg/f:DI 96)) [3 *i_5+0 S4 A32])
        (asm_operands:SI ("") ("=Q") 0 []
             []
             [] test.c:11)) "test.c":11:5 -1
     (expr_list:REG_DEAD (reg/v:SI 92 [ g ])
        (nil)))

Then LRA reloads the address and we ICE because we fail to recognize the
sign_extract outside the mem:

(insn 33 8 34 3 (set (reg:DI 100)
        (sign_extract:DI (ashift:DI (subreg:DI (reg/v:SI 92 [ g ]) 0)
                (const_int 2 [0x2]))
            (const_int 34 [0x22])
            (const_int 0 [0]))) "test.c":11:5 -1
     (nil))

The aarch64 changes here remove the support for this sign_extract
representation of addresses, fixing PR96998. Now this by itself would
regress code quality, so this change is paired with an improvement to
combine which prevents an extract rtx from being emitted in this case:
we now write the rtx above as a shift of an extend, which allows the
combination to go ahead.

Prior to this, combine.c:make_extraction() identified where we can emit
an ashift of an extend in place of an extraction, but failed to make the
corresponding canonicalization/simplification when presented with a mult
by a power of two. Such a representation is canonical when representing
a left-shifted address inside a mem.

This change remedies this situation. For rtxes such as:

(mult:DI (subreg:DI (reg:SI r) 0) (const_int 2^n))

where the bottom 32 + n bits are valid (the higher-order bits are
undefined) and make_extraction() is being asked to sign_extract the
lower (valid) bits, after the patch, we rewrite this as:

(mult:DI (sign_extend:DI (reg:SI r)) (const_int 2^n))

instead of using a sign_extract.

gcc/ChangeLog:

PR target/96998
* combine.c (make_extraction): Also handle shifts written as
(mult x 2^n), avoid creating an extract rtx for these.
* config/aarch64/aarch64.c (aarch64_is_extend_from_extract): Delete.
(aarch64_classify_index): Remove extract-based address handling.
(aarch64_strip_extend): Likewise.
(aarch64_rtx_arith_op_extract_p): Likewise, remove now-unused parameter.
Update callers...
(aarch64_rtx_costs): ... here.

gcc/testsuite/ChangeLog:

PR target/96998
* gcc.c-torture/compile/pr96998.c: New test.
gcc/combine.c
gcc/config/aarch64/aarch64.c
gcc/testsuite/gcc.c-torture/compile/pr96998.c [new file with mode: 0644]