This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/7] [ARC] Improves and fixes for small data support.
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Francois dot Bedard at synopsys dot com, sandra at codesourcery dot com, Claudiu Zissulescu <claziss at gmail dot com>
- Date: Tue, 15 Aug 2017 13:55:49 +0100
- Subject: Re: [PATCH 1/7] [ARC] Improves and fixes for small data support.
- Authentication-results: sourceware.org; auth=none
- References: <1500885779-12930-1-git-send-email-claziss@synopsys.com> <1500885779-12930-2-git-send-email-claziss@synopsys.com>
* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2017-07-24 10:42:53 +0200]:
> From: Claudiu Zissulescu <claziss@gmail.com>
>
> Add alignment check for short load/store instructions used for sdata,
> as they request 32-bit aligned short immediate. Use sdata symbol
> alignment information and emit scalled loads/stores whenever is
> possible. The scalled address will extend the access range for sdata
> symbols. Allow 64-bit datum into small data section, if double
> load/store instructions are present.
>
> gcc/
> 2017-04-12 Claudiu Zissulescu <claziss@synopsys.com>
>
> * config/arc/arc-protos.h (compact_sda_memory_operand): Update
> prototype.
> * config/arc/arc.c (arc_print_operand): Output scalled address for
> sdata whenever is possible.
> (arc_in_small_data_p): Allow sdata for 64bit datum when double
> load/stores are available.
> (compact_sda_memory_operand): Check for the alignment required by
> code density instructions.
> * config/arc/arc.md (movsi_insn): Use newly introduced Us0
> constraint.
> * config/arc/constraints.md (Usd): Update constraint.
> (Us0): New constraint.
> (Usc): Update constraint.
>
> gcc/testsuite/
> 2017-04-12 Claudiu Zissulescu <claziss@synopsys.com>
>
> * gcc.target/arc/sdata-3.c: New file.
Remember to add both new testsuite files in the ChangeLog, but
otherwise looks good.
Thanks,
Andrew
> ---
> gcc/config/arc/arc-protos.h | 2 +-
> gcc/config/arc/arc.c | 64 +++++++++++++++++++++++++++++-----
> gcc/config/arc/constraints.md | 4 +--
> gcc/testsuite/gcc.target/arc/sdata-3.c | 32 +++++++++++++++++
> gcc/testsuite/gcc.target/arc/sdata-4.c | 15 ++++++++
> 5 files changed, 105 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arc/sdata-3.c
> create mode 100644 gcc/testsuite/gcc.target/arc/sdata-4.c
>
> diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
> index 850795a..c831972 100644
> --- a/gcc/config/arc/arc-protos.h
> +++ b/gcc/config/arc/arc-protos.h
> @@ -27,7 +27,7 @@ extern struct rtx_def *gen_compare_reg (rtx, machine_mode);
> /* Declarations for various fns used in the .md file. */
> extern void arc_output_function_epilogue (FILE *, HOST_WIDE_INT, int);
> extern const char *output_shift (rtx *);
> -extern bool compact_sda_memory_operand (rtx op,machine_mode mode);
> +extern bool compact_sda_memory_operand (rtx, machine_mode, bool);
> extern bool arc_double_limm_p (rtx);
> extern void arc_print_operand (FILE *, rtx, int);
> extern void arc_print_operand_address (FILE *, rtx);
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index 89de6cd..091bc89 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -3900,6 +3900,26 @@ arc_print_operand (FILE *file, rtx x, int code)
> fputs (".as", file);
> output_scaled = 1;
> }
> + else if (LEGITIMATE_SMALL_DATA_ADDRESS_P (addr)
> + && GET_MODE_SIZE (GET_MODE (x)) > 1)
> + {
> + tree decl = NULL_TREE;
> + int align = 0;
> + if (GET_CODE (XEXP (addr, 1)) == SYMBOL_REF)
> + decl = SYMBOL_REF_DECL (XEXP (addr, 1));
> + else if (GET_CODE (XEXP (XEXP (XEXP (addr, 1), 0), 0))
> + == SYMBOL_REF)
> + decl = SYMBOL_REF_DECL (XEXP (XEXP (XEXP (addr, 1), 0), 0));
> + if (decl)
> + align = DECL_ALIGN (decl);
> + align = align / BITS_PER_UNIT;
> + if ((GET_MODE_SIZE (GET_MODE (x)) == 2)
> + && align && ((align & 1) == 0))
> + fputs (".as", file);
> + if ((GET_MODE_SIZE (GET_MODE (x)) >= 4)
> + && align && ((align & 3) == 0))
> + fputs (".as", file);
> + }
> break;
> case REG:
> break;
> @@ -7571,12 +7591,10 @@ arc_in_small_data_p (const_tree decl)
> {
> HOST_WIDE_INT size;
>
> + /* Strings and functions are never in small data area. */
> if (TREE_CODE (decl) == STRING_CST || TREE_CODE (decl) == FUNCTION_DECL)
> return false;
>
> -
> - /* We don't yet generate small-data references for -mabicalls. See related
> - -G handling in override_options. */
> if (TARGET_NO_SDATA_SET)
> return false;
>
> @@ -7595,7 +7613,7 @@ arc_in_small_data_p (const_tree decl)
> return true;
> }
> /* Only global variables go into sdata section for now. */
> - else if (1)
> + else
> {
> /* Don't put constants into the small data section: we want them
> to be in ROM rather than RAM. */
> @@ -7625,9 +7643,6 @@ arc_in_small_data_p (const_tree decl)
>
> size = int_size_in_bytes (TREE_TYPE (decl));
>
> -/* if (AGGREGATE_TYPE_P (TREE_TYPE (decl))) */
> -/* return false; */
> -
> /* Allow only <=4B long data types into sdata. */
> return (size > 0 && size <= 4);
> }
> @@ -7719,10 +7734,13 @@ small_data_pattern (rtx op, machine_mode)
> /* volatile cache option still to be handled. */
>
> bool
> -compact_sda_memory_operand (rtx op, machine_mode mode)
> +compact_sda_memory_operand (rtx op, machine_mode mode, bool short_p)
> {
> rtx addr;
> int size;
> + tree decl = NULL_TREE;
> + int align = 0;
> + int mask = 0;
>
> /* Eliminate non-memory operations. */
> if (GET_CODE (op) != MEM)
> @@ -7740,7 +7758,35 @@ compact_sda_memory_operand (rtx op, machine_mode mode)
> /* Decode the address now. */
> addr = XEXP (op, 0);
>
> - return LEGITIMATE_SMALL_DATA_ADDRESS_P (addr);
> + if (!LEGITIMATE_SMALL_DATA_ADDRESS_P (addr))
> + return false;
> +
> + if (!short_p || size == 1)
> + return true;
> +
> + /* Now check for the alignment, the short loads using gp require the
> + addresses to be aligned. */
> + if (GET_CODE (XEXP (addr, 1)) == SYMBOL_REF)
> + decl = SYMBOL_REF_DECL (XEXP (addr, 1));
> + else if (GET_CODE (XEXP (XEXP (XEXP (addr, 1), 0), 0)) == SYMBOL_REF)
> + decl = SYMBOL_REF_DECL (XEXP (XEXP (XEXP (addr, 1), 0), 0));
> + if (decl)
> + align = DECL_ALIGN (decl);
> + align = align / BITS_PER_UNIT;
> +
> + switch (mode)
> + {
> + case HImode:
> + mask = 1;
> + break;
> + default:
> + mask = 3;
> + break;
> + }
> +
> + if (align && ((align & mask) == 0))
> + return true;
> + return false;
> }
>
> /* Implement ASM_OUTPUT_ALIGNED_DECL_LOCAL. */
> diff --git a/gcc/config/arc/constraints.md b/gcc/config/arc/constraints.md
> index 6620daf..0ad318c 100644
> --- a/gcc/config/arc/constraints.md
> +++ b/gcc/config/arc/constraints.md
> @@ -355,7 +355,7 @@
> "@internal
> A valid _small-data_ memory operand for ARCompact instructions"
> (and (match_code "mem")
> - (match_test "compact_sda_memory_operand (op, VOIDmode)")))
> + (match_test "compact_sda_memory_operand (op, VOIDmode, true)")))
>
> (define_memory_constraint "Usc"
> "@internal
> @@ -363,7 +363,7 @@
> (and (match_code "mem")
> (match_test "!CONSTANT_P (XEXP (op,0))")
> ;; ??? the assembler rejects stores of immediates to small data.
> - (match_test "!compact_sda_memory_operand (op, VOIDmode)")))
> + (match_test "!compact_sda_memory_operand (op, VOIDmode, false)")))
>
> (define_constraint "Us<"
> "@internal
> diff --git a/gcc/testsuite/gcc.target/arc/sdata-3.c b/gcc/testsuite/gcc.target/arc/sdata-3.c
> new file mode 100644
> index 0000000..cdf3b6d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/sdata-3.c
> @@ -0,0 +1,32 @@
> +/* Check if sdata access is done correctly, specially
> + for variables which are having a different alignment
> + than the default data type indicates. */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +int g_a __attribute__ ((aligned (1)));
> +int g_b;
> +short g_c;
> +char g_d;
> +
> +#define TEST(name, optype) \
> + void test_ ## name (optype x) \
> + { \
> + g_ ## name += x; \
> + }
> +
> +TEST (a, int)
> +TEST (b, int)
> +TEST (c, short)
> +TEST (d, char)
> +
> +/* { dg-final { scan-assembler "ld r2,\\\[gp,@g_a@sda\\\]" } } */
> +/* { dg-final { scan-assembler "ld.as r2,\\\[gp,@g_b@sda\\\]" } } */
> +/* { dg-final { scan-assembler "ld\[hw\]\\\.as r2,\\\[gp,@g_c@sda\\\]" } } */
> +/* { dg-final { scan-assembler "ldb r2,\\\[gp,@g_d@sda\\\]" } } */
> +
> +/* { dg-final { scan-assembler "st r0,\\\[gp,@g_a@sda\\\]" } } */
> +/* { dg-final { scan-assembler "st_s r0,\\\[gp,@g_b@sda\\\]" { target { arcem || archs } } } } */
> +/* { dg-final { scan-assembler "st\\\.as r0,\\\[gp,@g_b@sda\\\]" { target { arc700 || arc6xx } } } } */
> +/* { dg-final { scan-assembler "st\[hw\]\\\.as r0,\\\[gp,@g_c@sda\\\]" } } */
> +/* { dg-final { scan-assembler "stb r0,\\\[gp,@g_d@sda\\\]" } } */
> diff --git a/gcc/testsuite/gcc.target/arc/sdata-4.c b/gcc/testsuite/gcc.target/arc/sdata-4.c
> new file mode 100644
> index 0000000..45fe712
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/sdata-4.c
> @@ -0,0 +1,15 @@
> +/* Check if sdata access is done correctly, specially
> + for variables which are having a different alignment
> + than the default data type indicates. */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +short gA __attribute__ ((aligned(1)));
> +
> +void foo (void)
> +{
> + gA += gA + 3;
> +}
> +
> +/* { dg-final { scan-assembler-not "ld\[wh\]_s r0,\\\[gp" } } */
> +/* { dg-final { scan-assembler-not "st\[wh\]\\\.as.*gp" } } */
> --
> 1.9.1
>