[PATCH PR95254] aarch64: gcc generate inefficient code with fixed sve vector length

[Richard Sandiford]

"Yangfei (Felix)" <felix.yang@huawei.com> writes:
> Turns out that this ICE triggers under x86 -m32.
>
> Command to reproduce:
> ~/build-gcc/x86_64-pc-linux-gnu/32/libgcc$ gcc  -g -O2 -m32 -O2  -g -O2 -DIN_GCC    -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include   -fpic -mlong-double-80 -DUSE_ELF_SYMVER -fcf-protection -mshstk -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector   -fpic -mlong-double-80 -DUSE_ELF_SYMVER -fcf-protection -mshstk -I. -I. -I../../.././gcc -I../../../../gcc-git/libgcc -I../../../../gcc-git/libgcc/. -I../../../../gcc-git/libgcc/../gcc -I../../../../gcc-git/libgcc/../include -I../../../../gcc-git/libgcc/config/libbid -DENABLE_DECIMAL_BID_FORMAT -DHAVE_CC_TLS  -DUSE_TLS -o _isinfd32.o -MT _isinfd32.o -MD -MP -MF _isinfd32.dep -c ../../../../gcc-git/libgcc/config/libbid/_isinfd32.c
>
> Source:
>  28 int
>  29 isinfd32 (_Decimal32 x) {
>  30   int res;
>  31   UINT64 x64;
>  32   union decimal32 ux;
>  33
>  34   ux.d = x;
>  35   x64 = __bid32_to_bid64 (ux.i);
>  36   res = __bid64_isInf (x64);
>  37   return (res);
>  38 }
>
> On the crash site (emit_single_push_insn), we have three insns:
> (gdb) p debug_rtx (prev)
> (insn 12 0 13 (parallel [
>             (set (reg/f:SI 7 sp)
>                 (plus:SI (reg/f:SI 7 sp)
>                     (const_int -12 [0xfffffffffffffff4])))
>             (clobber (reg:CC 17 flags))
>         ]) "../../../../gcc-git/libgcc/config/libbid/_isinfd32.c":35:9 -1
>      (expr_list:REG_ARGS_SIZE (const_int 12 [0xc])
>         (nil)))
> $1 = void
> (gdb) p debug_rtx (last)
> (insn 14 13 0 (set (mem:SI (reg/f:SI 87) [0  S4 A32])
>         (subreg:SI (reg/v:SD 86 [ x ]) 0)) "../../../../gcc-git/libgcc/config/libbid/_isinfd32.c":35:9 -1
>      (nil))
> $2 = void
> (gdb) p debug_rtx (PREV_INSN (last))
> (insn 13 12 14 (set (reg/f:SI 87)
>         (pre_dec:SI (reg/f:SI 7 sp))) "../../../../gcc-git/libgcc/config/libbid/_isinfd32.c":35:9 -1
>      (nil))
> $3 = void
>
> Here, insn 13 is invalid. It is emitted by: x = adjust_address (x, GET_MODE (y_inner), 0);
>
> The v5 patch attached addressed this issue.
>
> There two added changes compared with the v4 patch:
> 1. In candidate_mem_p, mov_optab for innermode should be available. 
>      In this case, mov_optab for SDmode is not there and subreg are added back by emit_move_insn_1.  So we won't get the benefit with the patch. 

I agree we should have this check.  I think the rule applies to all
of the transforms though, not just the mem one, so we should add
the check to the register and constant cases too.

> 2. Instead of using adjust_address, I changed to use adjust_address_nv to avoid the emit of invalid insn 13. 
>     The latter call to validize_mem() in emit_move_insn will take care of the address for us. 

The validation performed by validize_mem is the same as that performed
by adjust_address, so the only case this should make a difference is
for push_operands:

  /* If X or Y are memory references, verify that their addresses are valid
     for the machine.  */
  if (MEM_P (x)
      && (! memory_address_addr_space_p (GET_MODE (x), XEXP (x, 0),
					 MEM_ADDR_SPACE (x))
	  && ! push_operand (x, GET_MODE (x))))
    x = validize_mem (x);

  if (MEM_P (y)
      && ! memory_address_addr_space_p (GET_MODE (y), XEXP (y, 0),
					MEM_ADDR_SPACE (y)))
    y = validize_mem (y);

So I think the fix is to punt on push_operands instead (and continue
to use adjust_address rather than adjust_address_nv).

Thanks,
Richard