Re: [AArch64] Use arrays and loops rather than numbered variables in aarch64_operands_adjust_ok_for_ldpstp [1/2]

[Sudakshina <sudi dot das at arm dot com>]

Hi Jackson


On Tuesday 10 July 2018 09:37 AM, Jackson Woodruff wrote:
> Hi all,
>
> This patch removes some duplicated code.  Since this method deals with 
> four loads or stores, there is a lot of duplicated code that can 
> easily be replaced with smaller loops.
>
> Regtest and bootstrap OK.
>
> OK for trunk?
>
> Thanks,
>
> Jackson
>
> Changelog:
>
> gcc/
>
> 2018-06-28  Jackson Woodruff  <jackson.woodruff@arm.com>
>
>         * config/aarch64/aarch64.c 
> (aarch64_operands_adjust_ok_for_ldpstp):
>         Use arrays instead of numbered variables.
Thank you for doing this. This looks a lot neater now.
I am not a maintainer but I noticed a couple of nits:

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 
01f35f8e8525adb455780269757452c8c3eb20be..d0e9b2d464183eecc8cc7639ca3e981d2ff243ba 
100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -17026,23 +17026,21 @@ bool
 aarch64_operands_adjust_ok_for_ldpstp (rtx *operands, bool load,
                        scalar_mode mode)
 {
-  enum reg_class rclass_1, rclass_2, rclass_3, rclass_4;
-  HOST_WIDE_INT offvals[4], msize;
-  rtx mem_1, mem_2, mem_3, mem_4, reg_1, reg_2, reg_3, reg_4;
-  rtx base_1, base_2, base_3, base_4, offset_1, offset_2, offset_3, 
offset_4;
+  const int num_instructions = 4;
+  enum reg_class rclass[num_instructions];
+  HOST_WIDE_INT offvals[num_instructions], msize;
+  rtx mem[num_instructions], reg[num_instructions],
+      base[num_instructions], offset[num_instructions];
...
   /* Skip if memory operand is by itslef valid for ldp/stp.  */
-  if (!MEM_P (mem_1) || aarch64_mem_pair_operand (mem_1, mode))
+  if (!MEM_P (mem[1]) || aarch64_mem_pair_operand (mem[1], mode))

mem_1 == mem[1]?

     return false;

-  /* The mems cannot be volatile.  */
...

/* If we have SImode and slow unaligned ldp,
      check the alignment to be at least 8 byte. */
   if (mode == SImode
       && (aarch64_tune_params.extra_tuning_flags
-          & AARCH64_EXTRA_TUNE_SLOW_UNALIGNED_LDPW)
+      & AARCH64_EXTRA_TUNE_SLOW_UNALIGNED_LDPW)
       && !optimize_size
-      && MEM_ALIGN (mem_1) < 8 * BITS_PER_UNIT)
+      && MEM_ALIGN (mem[1]) < 8 * BITS_PER_UNIT)

Likewise
...
   /* Check if the registers are of same class.  */
-  if (rclass_1 != rclass_2 || rclass_2 != rclass_3 || rclass_3 != rclass_4)
-    return false;
+  for (int i = 0; i < 3; i++)

num_instructions -1 instead of 3 would be more consistent.

+    if (rclass[i] != rclass[i + 1])
+      return false;

It looks good otherwise.

Thanks
Sudi