Re: [PATCH v2] aarch64: Improve on ldp-stp policies code structure.

2023-09-29 Thread Philipp Tomsich 
Applied to master. Thanks!
--Philipp.

On Fri, 29 Sept 2023 at 12:34, Richard Sandiford
 wrote:
>
> Manos Anagnostakis  writes:
> > Improves on: 834fc2bf
> >
> > This improves the code structure of the ldp-stp policies
> > patch introduced in 834fc2bf
> >
> > Bootstrapped and regtested on aarch64-linux.
> >
> > gcc/ChangeLog:
> >   * config/aarch64/aarch64-opts.h (enum aarch64_ldp_policy): Removed.
> >   (enum aarch64_ldp_stp_policy): Merged enums aarch64_ldp_policy
> >   and aarch64_stp_policy to aarch64_ldp_stp_policy.
> >   (enum aarch64_stp_policy): Removed.
> >   * config/aarch64/aarch64-protos.h (struct tune_params): Removed
> >   aarch64_ldp_policy_model and aarch64_stp_policy_model enum types
> >   and left only the definitions to the aarch64-opts one.
> >   * config/aarch64/aarch64.cc (aarch64_parse_ldp_policy): Removed.
> >   (aarch64_parse_stp_policy): Removed.
> >   (aarch64_override_options_internal): Removed calls to parsing
> >   functions and added obvious direct assignments.
> >   (aarch64_mem_ok_with_ldpstp_policy_model): Improved
> >   code quality based on the new changes.
> >   * config/aarch64/aarch64.opt: Use single enum type
> >   aarch64_ldp_stp_policy for both ldp and stp options.
> >
> > gcc/testsuite/ChangeLog:
> >   * gcc.target/aarch64/ldp_aligned.c: Splitted into this and
> >   ldp_unaligned.
> >   * gcc.target/aarch64/stp_aligned.c: Splitted into this and
> >   stp_unaligned.
> >   * gcc.target/aarch64/ldp_unaligned.c: New test.
> >   * gcc.target/aarch64/stp_unaligned.c: New test.
>
> Nice!  OK for trunk, thanks.
>
> Sorry again for my mix-up with the original review.
>
> Richard
>
> > Signed-off-by: Manos Anagnostakis 
> > ---
> >  gcc/config/aarch64/aarch64-opts.h |  26 ++-
> >  gcc/config/aarch64/aarch64-protos.h   |  25 +--
> >  gcc/config/aarch64/aarch64.cc | 160 +++---
> >  gcc/config/aarch64/aarch64.opt|  29 +---
> >  .../gcc.target/aarch64/ldp_aligned.c  |  28 ---
> >  .../gcc.target/aarch64/ldp_unaligned.c|  40 +
> >  .../gcc.target/aarch64/stp_aligned.c  |  25 ---
> >  .../gcc.target/aarch64/stp_unaligned.c|  37 
> >  8 files changed, 155 insertions(+), 215 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/ldp_unaligned.c
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/stp_unaligned.c
> >
> > diff --git a/gcc/config/aarch64/aarch64-opts.h 
> > b/gcc/config/aarch64/aarch64-opts.h
> > index db8348507a3..831e28ab52a 100644
> > --- a/gcc/config/aarch64/aarch64-opts.h
> > +++ b/gcc/config/aarch64/aarch64-opts.h
> > @@ -108,20 +108,18 @@ enum aarch64_key_type {
> >AARCH64_KEY_B
> >  };
> >
> > -/* Load pair policy type.  */
> > -enum aarch64_ldp_policy {
> > -  LDP_POLICY_DEFAULT,
> > -  LDP_POLICY_ALWAYS,
> > -  LDP_POLICY_NEVER,
> > -  LDP_POLICY_ALIGNED
> > -};
> > -
> > -/* Store pair policy type.  */
> > -enum aarch64_stp_policy {
> > -  STP_POLICY_DEFAULT,
> > -  STP_POLICY_ALWAYS,
> > -  STP_POLICY_NEVER,
> > -  STP_POLICY_ALIGNED
> > +/* An enum specifying how to handle load and store pairs using
> > +   a fine-grained policy:
> > +   - LDP_STP_POLICY_DEFAULT: Use the policy defined in the tuning 
> > structure.
> > +   - LDP_STP_POLICY_ALIGNED: Emit ldp/stp if the source pointer is aligned
> > +   to at least double the alignment of the type.
> > +   - LDP_STP_POLICY_ALWAYS: Emit ldp/stp regardless of alignment.
> > +   - LDP_STP_POLICY_NEVER: Do not emit ldp/stp.  */
> > +enum aarch64_ldp_stp_policy {
> > +  AARCH64_LDP_STP_POLICY_DEFAULT,
> > +  AARCH64_LDP_STP_POLICY_ALIGNED,
> > +  AARCH64_LDP_STP_POLICY_ALWAYS,
> > +  AARCH64_LDP_STP_POLICY_NEVER
> >  };
> >
> >  #endif
> > diff --git a/gcc/config/aarch64/aarch64-protos.h 
> > b/gcc/config/aarch64/aarch64-protos.h
> > index 5c6802b4fe8..60a55f4bc19 100644
> > --- a/gcc/config/aarch64/aarch64-protos.h
> > +++ b/gcc/config/aarch64/aarch64-protos.h
> > @@ -568,30 +568,9 @@ struct tune_params
> >/* Place prefetch struct pointer at the end to enable type checking
> >   errors when tune_params misses elements (e.g., from erroneous 
> > merges).  */
> >const struct cpu_prefetch_tune *prefetch;
> > -/* An enum specifying how to handle load pairs using a fine-grained policy:
> > -   - LDP_POLICY_ALIGNED: Emit ldp if the source pointer is aligned
> > -   to at least double the alignment of the type.
> > -   - LDP_POLICY_ALWAYS: Emit ldp regardless of alignment.
> > -   - LDP_POLICY_NEVER: Do not emit ldp.  */
> >
> > -  enum aarch64_ldp_policy_model
> > -  {
> > -LDP_POLICY_ALIGNED,
> > -LDP_POLICY_ALWAYS,
> > -LDP_POLICY_NEVER
> > -  } ldp_policy_model;
> > -/* An enum specifying how to handle store pairs using a fine-grained 
> > policy:
> > -   - STP_POLICY_ALIGNED: Emit stp if the source pointer is aligned
> > -   to at least double the alignment of the type.
> > -   -

Re: [PATCH v2] aarch64: Improve on ldp-stp policies code structure.

2023-09-29 Thread Richard Sandiford 
Manos Anagnostakis  writes:
> Improves on: 834fc2bf
>
> This improves the code structure of the ldp-stp policies
> patch introduced in 834fc2bf
>
> Bootstrapped and regtested on aarch64-linux.
>
> gcc/ChangeLog:
>   * config/aarch64/aarch64-opts.h (enum aarch64_ldp_policy): Removed.
>   (enum aarch64_ldp_stp_policy): Merged enums aarch64_ldp_policy
>   and aarch64_stp_policy to aarch64_ldp_stp_policy.
>   (enum aarch64_stp_policy): Removed.
>   * config/aarch64/aarch64-protos.h (struct tune_params): Removed
>   aarch64_ldp_policy_model and aarch64_stp_policy_model enum types
>   and left only the definitions to the aarch64-opts one.
>   * config/aarch64/aarch64.cc (aarch64_parse_ldp_policy): Removed.
>   (aarch64_parse_stp_policy): Removed.
>   (aarch64_override_options_internal): Removed calls to parsing
>   functions and added obvious direct assignments.
>   (aarch64_mem_ok_with_ldpstp_policy_model): Improved
>   code quality based on the new changes.
>   * config/aarch64/aarch64.opt: Use single enum type
>   aarch64_ldp_stp_policy for both ldp and stp options.
>
> gcc/testsuite/ChangeLog:
>   * gcc.target/aarch64/ldp_aligned.c: Splitted into this and
>   ldp_unaligned.
>   * gcc.target/aarch64/stp_aligned.c: Splitted into this and
>   stp_unaligned.
>   * gcc.target/aarch64/ldp_unaligned.c: New test.
>   * gcc.target/aarch64/stp_unaligned.c: New test.

Nice!  OK for trunk, thanks.

Sorry again for my mix-up with the original review.

Richard

> Signed-off-by: Manos Anagnostakis 
> ---
>  gcc/config/aarch64/aarch64-opts.h |  26 ++-
>  gcc/config/aarch64/aarch64-protos.h   |  25 +--
>  gcc/config/aarch64/aarch64.cc | 160 +++---
>  gcc/config/aarch64/aarch64.opt|  29 +---
>  .../gcc.target/aarch64/ldp_aligned.c  |  28 ---
>  .../gcc.target/aarch64/ldp_unaligned.c|  40 +
>  .../gcc.target/aarch64/stp_aligned.c  |  25 ---
>  .../gcc.target/aarch64/stp_unaligned.c|  37 
>  8 files changed, 155 insertions(+), 215 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/ldp_unaligned.c
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/stp_unaligned.c
>
> diff --git a/gcc/config/aarch64/aarch64-opts.h 
> b/gcc/config/aarch64/aarch64-opts.h
> index db8348507a3..831e28ab52a 100644
> --- a/gcc/config/aarch64/aarch64-opts.h
> +++ b/gcc/config/aarch64/aarch64-opts.h
> @@ -108,20 +108,18 @@ enum aarch64_key_type {
>AARCH64_KEY_B
>  };
>  
> -/* Load pair policy type.  */
> -enum aarch64_ldp_policy {
> -  LDP_POLICY_DEFAULT,
> -  LDP_POLICY_ALWAYS,
> -  LDP_POLICY_NEVER,
> -  LDP_POLICY_ALIGNED
> -};
> -
> -/* Store pair policy type.  */
> -enum aarch64_stp_policy {
> -  STP_POLICY_DEFAULT,
> -  STP_POLICY_ALWAYS,
> -  STP_POLICY_NEVER,
> -  STP_POLICY_ALIGNED
> +/* An enum specifying how to handle load and store pairs using
> +   a fine-grained policy:
> +   - LDP_STP_POLICY_DEFAULT: Use the policy defined in the tuning structure.
> +   - LDP_STP_POLICY_ALIGNED: Emit ldp/stp if the source pointer is aligned
> +   to at least double the alignment of the type.
> +   - LDP_STP_POLICY_ALWAYS: Emit ldp/stp regardless of alignment.
> +   - LDP_STP_POLICY_NEVER: Do not emit ldp/stp.  */
> +enum aarch64_ldp_stp_policy {
> +  AARCH64_LDP_STP_POLICY_DEFAULT,
> +  AARCH64_LDP_STP_POLICY_ALIGNED,
> +  AARCH64_LDP_STP_POLICY_ALWAYS,
> +  AARCH64_LDP_STP_POLICY_NEVER
>  };
>  
>  #endif
> diff --git a/gcc/config/aarch64/aarch64-protos.h 
> b/gcc/config/aarch64/aarch64-protos.h
> index 5c6802b4fe8..60a55f4bc19 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -568,30 +568,9 @@ struct tune_params
>/* Place prefetch struct pointer at the end to enable type checking
>   errors when tune_params misses elements (e.g., from erroneous merges).  
> */
>const struct cpu_prefetch_tune *prefetch;
> -/* An enum specifying how to handle load pairs using a fine-grained policy:
> -   - LDP_POLICY_ALIGNED: Emit ldp if the source pointer is aligned
> -   to at least double the alignment of the type.
> -   - LDP_POLICY_ALWAYS: Emit ldp regardless of alignment.
> -   - LDP_POLICY_NEVER: Do not emit ldp.  */
>  
> -  enum aarch64_ldp_policy_model
> -  {
> -LDP_POLICY_ALIGNED,
> -LDP_POLICY_ALWAYS,
> -LDP_POLICY_NEVER
> -  } ldp_policy_model;
> -/* An enum specifying how to handle store pairs using a fine-grained policy:
> -   - STP_POLICY_ALIGNED: Emit stp if the source pointer is aligned
> -   to at least double the alignment of the type.
> -   - STP_POLICY_ALWAYS: Emit stp regardless of alignment.
> -   - STP_POLICY_NEVER: Do not emit stp.  */
> -
> -  enum aarch64_stp_policy_model
> -  {
> -STP_POLICY_ALIGNED,
> -STP_POLICY_ALWAYS,
> -STP_POLICY_NEVER
> -  } stp_policy_model;
> +  /* Define models for the aarch64_ldp_stp_policy.  */
> +  enum

[PATCH v2] aarch64: Improve on ldp-stp policies code structure.

2023-09-29 Thread Manos Anagnostakis 
Improves on: 834fc2bf

This improves the code structure of the ldp-stp policies
patch introduced in 834fc2bf

Bootstrapped and regtested on aarch64-linux.

gcc/ChangeLog:
* config/aarch64/aarch64-opts.h (enum aarch64_ldp_policy): Removed.
(enum aarch64_ldp_stp_policy): Merged enums aarch64_ldp_policy
and aarch64_stp_policy to aarch64_ldp_stp_policy.
(enum aarch64_stp_policy): Removed.
* config/aarch64/aarch64-protos.h (struct tune_params): Removed
aarch64_ldp_policy_model and aarch64_stp_policy_model enum types
and left only the definitions to the aarch64-opts one.
* config/aarch64/aarch64.cc (aarch64_parse_ldp_policy): Removed.
(aarch64_parse_stp_policy): Removed.
(aarch64_override_options_internal): Removed calls to parsing
functions and added obvious direct assignments.
(aarch64_mem_ok_with_ldpstp_policy_model): Improved
code quality based on the new changes.
* config/aarch64/aarch64.opt: Use single enum type
aarch64_ldp_stp_policy for both ldp and stp options.

gcc/testsuite/ChangeLog:
* gcc.target/aarch64/ldp_aligned.c: Splitted into this and
ldp_unaligned.
* gcc.target/aarch64/stp_aligned.c: Splitted into this and
stp_unaligned.
* gcc.target/aarch64/ldp_unaligned.c: New test.
* gcc.target/aarch64/stp_unaligned.c: New test.

Signed-off-by: Manos Anagnostakis 
---
 gcc/config/aarch64/aarch64-opts.h |  26 ++-
 gcc/config/aarch64/aarch64-protos.h   |  25 +--
 gcc/config/aarch64/aarch64.cc | 160 +++---
 gcc/config/aarch64/aarch64.opt|  29 +---
 .../gcc.target/aarch64/ldp_aligned.c  |  28 ---
 .../gcc.target/aarch64/ldp_unaligned.c|  40 +
 .../gcc.target/aarch64/stp_aligned.c  |  25 ---
 .../gcc.target/aarch64/stp_unaligned.c|  37 
 8 files changed, 155 insertions(+), 215 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/ldp_unaligned.c
 create mode 100644 gcc/testsuite/gcc.target/aarch64/stp_unaligned.c

diff --git a/gcc/config/aarch64/aarch64-opts.h 
b/gcc/config/aarch64/aarch64-opts.h
index db8348507a3..831e28ab52a 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -108,20 +108,18 @@ enum aarch64_key_type {
   AARCH64_KEY_B
 };
 
-/* Load pair policy type.  */
-enum aarch64_ldp_policy {
-  LDP_POLICY_DEFAULT,
-  LDP_POLICY_ALWAYS,
-  LDP_POLICY_NEVER,
-  LDP_POLICY_ALIGNED
-};
-
-/* Store pair policy type.  */
-enum aarch64_stp_policy {
-  STP_POLICY_DEFAULT,
-  STP_POLICY_ALWAYS,
-  STP_POLICY_NEVER,
-  STP_POLICY_ALIGNED
+/* An enum specifying how to handle load and store pairs using
+   a fine-grained policy:
+   - LDP_STP_POLICY_DEFAULT: Use the policy defined in the tuning structure.
+   - LDP_STP_POLICY_ALIGNED: Emit ldp/stp if the source pointer is aligned
+   to at least double the alignment of the type.
+   - LDP_STP_POLICY_ALWAYS: Emit ldp/stp regardless of alignment.
+   - LDP_STP_POLICY_NEVER: Do not emit ldp/stp.  */
+enum aarch64_ldp_stp_policy {
+  AARCH64_LDP_STP_POLICY_DEFAULT,
+  AARCH64_LDP_STP_POLICY_ALIGNED,
+  AARCH64_LDP_STP_POLICY_ALWAYS,
+  AARCH64_LDP_STP_POLICY_NEVER
 };
 
 #endif
diff --git a/gcc/config/aarch64/aarch64-protos.h 
b/gcc/config/aarch64/aarch64-protos.h
index 5c6802b4fe8..60a55f4bc19 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -568,30 +568,9 @@ struct tune_params
   /* Place prefetch struct pointer at the end to enable type checking
  errors when tune_params misses elements (e.g., from erroneous merges).  */
   const struct cpu_prefetch_tune *prefetch;
-/* An enum specifying how to handle load pairs using a fine-grained policy:
-   - LDP_POLICY_ALIGNED: Emit ldp if the source pointer is aligned
-   to at least double the alignment of the type.
-   - LDP_POLICY_ALWAYS: Emit ldp regardless of alignment.
-   - LDP_POLICY_NEVER: Do not emit ldp.  */
 
-  enum aarch64_ldp_policy_model
-  {
-LDP_POLICY_ALIGNED,
-LDP_POLICY_ALWAYS,
-LDP_POLICY_NEVER
-  } ldp_policy_model;
-/* An enum specifying how to handle store pairs using a fine-grained policy:
-   - STP_POLICY_ALIGNED: Emit stp if the source pointer is aligned
-   to at least double the alignment of the type.
-   - STP_POLICY_ALWAYS: Emit stp regardless of alignment.
-   - STP_POLICY_NEVER: Do not emit stp.  */
-
-  enum aarch64_stp_policy_model
-  {
-STP_POLICY_ALIGNED,
-STP_POLICY_ALWAYS,
-STP_POLICY_NEVER
-  } stp_policy_model;
+  /* Define models for the aarch64_ldp_stp_policy.  */
+  enum aarch64_ldp_stp_policy ldp_policy_model, stp_policy_model;
 };
 
 /* Classifies an address.
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index aa920fc703a..dcded70c981 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -1358,8 +1358,8 @@ static const struct