On 3/25/2022 4:17 AM, Jakub Jelinek wrote:
Hi!
The following testcase ICEs on aarch64-linux with -g and
assembles with a warning otherwise, because it emits
ldrb w0,[x0,16]!
instruction which sets the x0 register multiple times.
Due to disabled DCE (from -Og) we end up before REE with:
(insn 12 39 13 2 (set (reg:SI 1 x1 [orig:93 _2 ] [93])
(zero_extend:SI (mem/c:QI (pre_modify:DI (reg/f:DI 0 x0 [114])
(plus:DI (reg/f:DI 0 x0 [114])
(const_int 16 [0x10]))) [1 u128_1+0 S1 A128])))
"pr103775.c":5:35 117 {*zero_extendqisi2_aarch64}
(expr_list:REG_INC (reg/f:DI 0 x0 [114])
(nil)))
(insn 13 12 14 2 (set (reg:DI 0 x0 [orig:112 _2 ] [112])
(zero_extend:DI (reg:SI 1 x1 [orig:93 _2 ] [93]))) "pr103775.c":5:16
111 {*zero_extendsidi2_aarch64}
(nil))
which is valid but not exactly efficient as x0 is dead after the
insn that auto-increments it. REE turns it into:
(insn 12 39 44 2 (set (reg:DI 0 x0)
(zero_extend:DI (mem/c:QI (pre_modify:DI (reg/f:DI 0 x0 [114])
(plus:DI (reg/f:DI 0 x0 [114])
(const_int 16 [0x10]))) [1 u128_1+0 S1 A128])))
"pr103775.c":5:35 119 {*zero_extendqidi2_aarch64}
(expr_list:REG_INC (reg/f:DI 0 x0 [114])
(nil)))
(insn 44 12 14 2 (set (reg:DI 1 x1)
(reg:DI 0 x0)) "pr103775.c":5:35 -1
(nil))
which is invalid because it sets x0 multiple times, one
in SET_DEST of the PATTERN and once in PRE_MODIFY.
As perhaps other passes than REE might suffer from it, IMHO it is better
to reject this during change validation.
Below is one patch that does that only if reload_completed, attached
is another version that does it always even before reload.
I've so far bootstrapped/regtested the first patch on
{x86_64,i686,powerpc64le,armv7hl}-linux, aarch64-linux regtest
is still pending.
If you prefer the second version, I can start testing it momentarily.
2022-03-25 Jakub Jelinek<ja...@redhat.com>
PR rtl-optimization/103775
* recog.cc (check_invalid_inc_dec): New function.
(insn_invalid_p): Return 1 if REG_INC operand overlaps
any stored REGs.
* gcc.dg/pr103775.c: New test.
OK. I bet this could also be extended to catch the case where the
autoinc'd operand is used as a source elsewhere in the insn. That's
supposed to be avoided in RTL, but we don't check it and I've seen it
sneak in (see PR 101697):
(insn 1444 1443 164 31 (parallel [
(set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [3 S4 A32])
(reg/f:SI 7 sp))
(clobber (reg:CC 12 cc))
]) "libc/inet/getaddrinfo.c":466:11 -1
(expr_list:REG_ARGS_SIZE (const_int 4 [0x4])
(nil)))
While this has well defined semantics on the H8 RTL considers it
ill-defined, but never checks for it.
If a register used as the operand of these expressions is used in
another address in an insn, the original value of the register is used.
Uses of the register outside of an address are not permitted within the
same insn as a use in an embedded side effect expression because such
insns behave differently on different machines and hence must be treated
as ambiguous and disallowed.
Jeff
