[PATCH][ARM] Fix Thumb-1 ldm (PR89190)

[Kyrill Tkachov]

Hi Wilco,

On 2/13/19 12:42 PM, Wilco Dijkstra wrote:
>
> ping
>
>
> From: Wilco Dijkstra
> Sent: 04 February 2019 14:59
> To: GCC Patches
> Cc: nd; Kyrylo Tkachov
> Subject: [PATCH][ARM] Fix Thumb-1 ldm (PR89190)
>
>
> This patch fixes an ICE in the Thumb-1 LDM peepholer. Thumb-1 LDMs
> always update the base register except if the base is loaded.
> The current implementation rejects LDMs where the base is not dead,
> however this doesn't exclude the case where the base is loaded as
> well as dead.  Fix this by explicitly checking whether the base is
> loaded.  Also enable LDMs which load the first register.
>
> Bootstrap OK on armhf, testsuite passes. OK for commit?
>
Ok.

Thanks,

Kyrill


> ChangeLog:
> 2019-02-04  Wilco Dijkstra  <wdijkstr@arm.com>
>
> Â Â Â Â Â Â Â  PR target/89190
> Â Â Â Â Â Â Â  * config/arm/arm.c (ldm_stm_operation_p) Set
> Â Â Â Â Â Â Â  addr_reg_in_reglist correctly for first register.
> Â Â Â Â Â Â Â  (load_multiple_sequence): Remove dead base check.
> Â Â Â Â Â Â Â  (gen_ldm_seq): Correctly set write_back for Thumb-1.
>
> Â Â Â Â Â Â Â  PR target/89190
> Â Â Â Â Â Â Â  * gcc.target/arm/pr89190.c: New test.
> --
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 
> 16e22eed871ca34d56c83c334688e5a95970638e..c4c9b4a667100d81d918196713e40b01ee232ee2 
> 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -13191,6 +13191,9 @@ ldm_stm_operation_p (rtx op, bool load, 
> machine_mode mode,
> Â Â  if (load && (REGNO (reg) == SP_REGNUM) && (REGNO (addr) != SP_REGNUM))
> Â Â Â Â  return false;
>
> +Â  if (regno == REGNO (addr))
> +Â Â Â  addr_reg_in_reglist = true;
> +
> Â Â  for (; i < count; i++)
> Â Â Â Â  {
> Â Â Â Â Â Â  elt = XVECEXP (op, 0, i);
> @@ -13385,7 +13388,6 @@ load_multiple_sequence (rtx *operands, int 
> nops, int *regs, int *saved_order,
> Â Â  int unsorted_regs[MAX_LDM_STM_OPS];
> Â Â  HOST_WIDE_INT unsorted_offsets[MAX_LDM_STM_OPS];
> Â Â  int order[MAX_LDM_STM_OPS];
> -Â  rtx base_reg_rtx = NULL;
> Â Â  int base_reg = -1;
> Â Â  int i, ldm_case;
>
> @@ -13430,7 +13432,6 @@ load_multiple_sequence (rtx *operands, int 
> nops, int *regs, int *saved_order,
> Â Â Â Â Â Â Â Â Â Â  if (i == 0)
> Â Â Â Â Â Â Â Â Â Â Â Â  {
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â  base_reg = REGNO (reg);
> -Â Â Â Â Â Â Â Â Â Â Â Â  base_reg_rtx = reg;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â  if (TARGET_THUMB1 && base_reg > LAST_LO_REGNUM)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  return 0;
> Â Â Â Â Â Â Â Â Â Â Â Â  }
> @@ -13489,10 +13490,6 @@ load_multiple_sequence (rtx *operands, int 
> nops, int *regs, int *saved_order,
> Â Â Â Â Â Â  *load_offset = unsorted_offsets[order[0]];
> Â Â Â Â  }
>
> -Â  if (TARGET_THUMB1
> -Â Â Â Â Â  && !peep2_reg_dead_p (nops, base_reg_rtx))
> -Â Â Â  return 0;
> -
> Â Â  if (unsorted_offsets[order[0]] == 0)
> Â Â Â Â  ldm_case = 1; /* ldmia */
> Â Â  else if (TARGET_ARM && unsorted_offsets[order[0]] == 4)
> @@ -13868,9 +13865,17 @@ gen_ldm_seq (rtx *operands, int nops, bool 
> sort_regs)
>
> Â Â  if (TARGET_THUMB1)
> Â Â Â Â  {
> -Â Â Â Â Â  gcc_assert (peep2_reg_dead_p (nops, base_reg_rtx));
> Â Â Â Â Â Â  gcc_assert (ldm_case == 1 || ldm_case == 5);
> -Â Â Â Â Â  write_back = TRUE;
> +
> +      /* Thumb-1 ldm uses writeback except if the base is loaded.  */
> +Â Â Â Â Â  write_back = true;
> +Â Â Â Â Â  for (i = 0; i < nops; i++)
> +Â Â Â Â Â Â  if (base_reg == regs[i])
> +Â Â Â Â Â Â Â Â  write_back = false;
> +
> +      /* Ensure the base is dead if it is updated.  */
> +Â Â Â Â Â  if (write_back && !peep2_reg_dead_p (nops, base_reg_rtx))
> +Â Â Â Â Â Â  return false;
> Â Â Â Â  }
>
> Â Â  if (ldm_case == 5)
> @@ -13878,8 +13883,7 @@ gen_ldm_seq (rtx *operands, int nops, bool 
> sort_regs)
> Â Â Â Â Â Â  rtx newbase = TARGET_THUMB1 ? base_reg_rtx : gen_rtx_REG 
> (SImode, regs[0]);
> Â Â Â Â Â Â  emit_insn (gen_addsi3 (newbase, base_reg_rtx, GEN_INT (offset)));
> Â Â Â Â Â Â  offset = 0;
> -Â Â Â Â Â  if (!TARGET_THUMB1)
> -Â Â Â Â Â Â  base_reg_rtx = newbase;
> +Â Â Â Â Â  base_reg_rtx = newbase;
> Â Â Â Â  }
>
> Â Â  for (i = 0; i < nops; i++)
> diff --git a/gcc/testsuite/gcc.target/arm/pr89190.c 
> b/gcc/testsuite/gcc.target/arm/pr89190.c
> new file mode 100644
> index 
> 0000000000000000000000000000000000000000..e622d7081ed01a6318e2c4b3e07598d495bd7e0f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr89190.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_arch_v8m_base_ok } */
> +/* { dg-options "-O2" } */
> +/* { dg-add-options arm_arch_v8m_base } */
> +
> +long long a;
> +int b, c;
> +int d(int e, int f) { return e << f; }
> +void g() {
> +Â  long long h;
> +Â  char i = d(b >= 7, 2);
> +Â  c = i == 0 ?: 1 / i;
> +Â  h = c && a ?: c + a;
> +Â  b = h;
> +}