This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] rs6000: Improve fusion assembler output
- From: Michael Meissner <meissner at linux dot vnet dot ibm dot com>
- To: Segher Boessenkool <segher at kernel dot crashing dot org>
- Cc: gcc-patches at gcc dot gnu dot org, dje dot gcc at gmail dot com, meissner at gcc dot gnu dot org
- Date: Thu, 30 Nov 2017 12:54:53 -0500
- Subject: Re: [PATCH] rs6000: Improve fusion assembler output
- Authentication-results: sourceware.org; auth=none
- References: <15d8c743179eaeb9382ea63f183a19a56d4d807b.1512041813.git.segher@kernel.crashing.org>
On Thu, Nov 30, 2017 at 11:59:37AM +0000, Segher Boessenkool wrote:
> This improves the output for load and store fusion a little. In most
> cases it removes the comment output, because that makes the generated
> assembler code hard to read, and equivalent info is available with -dp
> anyway. For the vector loads it puts the comment on the second insn,
> where it doesn't interfere with other debug comments.
>
> Mike, does this look good? Or is there something I'm missing :-)
>
> Tested on powerpc64-linux {-m32,-m64}.
The comment was used by my perl script (analyze-ppc-asm) that looks at .s files
and prints out statistics. I can adjust the tool so it no longer looks for the
comment, but actually looks at the adjacent instructions (which I do in a few
other cases).
>
> Segher
>
>
> 2017-11-30 Segher Boessenkool <segher@kernel.crashing.org>
>
> * config/rs6000/rs6000-protos.h (emit_fusion_addis): Remove last two
> parameters from prototype.
> * config/rs6000/rs6000.c (emit_fusion_addis): Remove last two
> parameters. Don't print a comment.
> (emit_fusion_gpr_load): Adjust.
> (emit_fusion_load_store): Adjust.
> * config/rs6000/rs6000.md (*fusion_p9_<mode>_constant): Adjust.
> * config/rs6000/vsx.md (two peepholes): Print the "vector load fusion"
> comment on the second line.
>
> gcc/testsuite/
> * gcc.target/powerpc/fusion.c: Add -dp to options. Adjust the expected
> output.
> * gcc.target/powerpc/fusion3.c: Ditto.
> * gcc.target/powerpc/fusion4.c: Ditto.
>
> ---
> gcc/config/rs6000/rs6000-protos.h | 2 +-
> gcc/config/rs6000/rs6000.c | 23 +++++------------------
> gcc/config/rs6000/rs6000.md | 2 +-
> gcc/config/rs6000/vsx.md | 4 ++--
> gcc/testsuite/gcc.target/powerpc/fusion.c | 4 ++--
> gcc/testsuite/gcc.target/powerpc/fusion3.c | 10 +++++-----
> gcc/testsuite/gcc.target/powerpc/fusion4.c | 5 ++---
> 7 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
> index f3f5f27..9a16a51 100644
> --- a/gcc/config/rs6000/rs6000-protos.h
> +++ b/gcc/config/rs6000/rs6000-protos.h
> @@ -96,7 +96,7 @@ extern bool quad_address_p (rtx, machine_mode, bool);
> extern bool quad_load_store_p (rtx, rtx);
> extern bool fusion_gpr_load_p (rtx, rtx, rtx, rtx);
> extern void expand_fusion_gpr_load (rtx *);
> -extern void emit_fusion_addis (rtx, rtx, const char *, const char *);
> +extern void emit_fusion_addis (rtx, rtx);
> extern void emit_fusion_load_store (rtx, rtx, rtx, const char *);
> extern const char *emit_fusion_gpr_load (rtx, rtx);
> extern bool fusion_p9_p (rtx, rtx, rtx, rtx);
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 2948f2a..9929a45 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -38689,16 +38689,10 @@ expand_fusion_gpr_load (rtx *operands)
> sequence. */
>
> void
> -emit_fusion_addis (rtx target, rtx addis_value, const char *comment,
> - const char *mode_name)
> +emit_fusion_addis (rtx target, rtx addis_value)
> {
> rtx fuse_ops[10];
> - char insn_template[80];
> const char *addis_str = NULL;
> - const char *comment_str = ASM_COMMENT_START;
> -
> - if (*comment_str == ' ')
> - comment_str++;
>
> /* Emit the addis instruction. */
> fuse_ops[0] = target;
> @@ -38778,9 +38772,7 @@ emit_fusion_addis (rtx target, rtx addis_value, const char *comment,
> if (!addis_str)
> fatal_insn ("Could not generate addis value for fusion", addis_value);
>
> - sprintf (insn_template, "%s\t\t%s %s, type %s", addis_str, comment_str,
> - comment, mode_name);
> - output_asm_insn (insn_template, fuse_ops);
> + output_asm_insn (addis_str, fuse_ops);
> }
>
> /* Emit a D-form load or store instruction that is the second instruction
> @@ -38913,7 +38905,6 @@ emit_fusion_gpr_load (rtx target, rtx mem)
> rtx addr;
> rtx load_offset;
> const char *load_str = NULL;
> - const char *mode_name = NULL;
> machine_mode mode;
>
> if (GET_CODE (mem) == ZERO_EXTEND)
> @@ -38929,25 +38920,21 @@ emit_fusion_gpr_load (rtx target, rtx mem)
> switch (mode)
> {
> case E_QImode:
> - mode_name = "char";
> load_str = "lbz";
> break;
>
> case E_HImode:
> - mode_name = "short";
> load_str = "lhz";
> break;
>
> case E_SImode:
> case E_SFmode:
> - mode_name = (mode == SFmode) ? "float" : "int";
> load_str = "lwz";
> break;
>
> case E_DImode:
> case E_DFmode:
> gcc_assert (TARGET_POWERPC64);
> - mode_name = (mode == DFmode) ? "double" : "long";
> load_str = "ld";
> break;
>
> @@ -38956,7 +38943,7 @@ emit_fusion_gpr_load (rtx target, rtx mem)
> }
>
> /* Emit the addis instruction. */
> - emit_fusion_addis (target, addis_value, "gpr load fusion", mode_name);
> + emit_fusion_addis (target, addis_value);
>
> /* Emit the D-form load instruction. */
> emit_fusion_load_store (target, target, load_offset, load_str);
> @@ -39225,7 +39212,7 @@ emit_fusion_p9_load (rtx reg, rtx mem, rtx tmp_reg)
> fusion_split_address (addr, &hi, &lo);
>
> /* Emit the addis instruction. */
> - emit_fusion_addis (tmp_reg, hi, "power9 load fusion", GET_MODE_NAME (mode));
> + emit_fusion_addis (tmp_reg, hi);
>
> /* Emit the D-form load instruction. */
> emit_fusion_load_store (reg, tmp_reg, lo, load_string);
> @@ -39312,7 +39299,7 @@ emit_fusion_p9_store (rtx mem, rtx reg, rtx tmp_reg)
> fusion_split_address (addr, &hi, &lo);
>
> /* Emit the addis instruction. */
> - emit_fusion_addis (tmp_reg, hi, "power9 store fusion", GET_MODE_NAME (mode));
> + emit_fusion_addis (tmp_reg, hi);
>
> /* Emit the D-form load instruction. */
> emit_fusion_load_store (reg, tmp_reg, lo, store_string);
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 1c15665..7e91d54 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -14120,7 +14120,7 @@ (define_insn "*fusion_p9_<mode>_constant"
> UNSPEC_FUSION_P9))]
> "TARGET_P9_FUSION"
> {
> - emit_fusion_addis (operands[0], operands[1], "constant", "<MODE>");
> + emit_fusion_addis (operands[0], operands[1]);
> return "ori %0,%0,%2";
> }
> [(set_attr "type" "two")
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 00d7656..f6f2bd4 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -4160,7 +4160,7 @@ (define_peephole
> (mem:VSX_M (plus:P (match_dup 0)
> (match_operand:P 3 "int_reg_operand" ""))))]
> "TARGET_VSX && TARGET_P8_FUSION && !TARGET_P9_VECTOR"
> - "li %0,%1\t\t\t# vector load fusion\;lx<VSX_M:VSm>x %x2,%0,%3"
> + "li %0,%1\;lx<VSX_M:VSm>x %x2,%0,%3\t\t\t# vector load fusion"
> [(set_attr "length" "8")
> (set_attr "type" "vecload")])
>
> @@ -4171,7 +4171,7 @@ (define_peephole
> (mem:VSX_M (plus:P (match_operand:P 3 "int_reg_operand" "")
> (match_dup 0))))]
> "TARGET_VSX && TARGET_P8_FUSION && !TARGET_P9_VECTOR"
> - "li %0,%1\t\t\t# vector load fusion\;lx<VSX_M:VSm>x %x2,%0,%3"
> + "li %0,%1\;lx<VSX_M:VSm>x %x2,%0,%3\t\t\t# vector load fusion"
> [(set_attr "length" "8")
> (set_attr "type" "vecload")])
>
> diff --git a/gcc/testsuite/gcc.target/powerpc/fusion.c b/gcc/testsuite/gcc.target/powerpc/fusion.c
> index 83dbddc..3c75be4 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fusion.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fusion.c
> @@ -2,7 +2,7 @@
> /* { dg-skip-if "" { powerpc*-*-darwin* } } */
> /* { dg-require-effective-target powerpc_p8vector_ok } */
> /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
> -/* { dg-options "-mcpu=power7 -mtune=power8 -O3" } */
> +/* { dg-options "-mcpu=power7 -mtune=power8 -O3 -dp" } */
>
> #define LARGE 0x12345
>
> @@ -13,7 +13,7 @@ int fusion_short (short *p){ return p[LARGE]; }
> int fusion_int (int *p){ return p[LARGE]; }
> unsigned fusion_uns (unsigned *p){ return p[LARGE]; }
>
> -/* { dg-final { scan-assembler-times "gpr load fusion" 6 } } */
> +/* { dg-final { scan-assembler-times "fusion_gpr_load" 6 } } */
> /* { dg-final { scan-assembler-times "lbz" 2 } } */
> /* { dg-final { scan-assembler-times "extsb" 1 } } */
> /* { dg-final { scan-assembler-times "lhz" 2 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fusion3.c b/gcc/testsuite/gcc.target/powerpc/fusion3.c
> index bb35ccd..0d9f318 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fusion3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fusion3.c
> @@ -2,7 +2,7 @@
> /* { dg-skip-if "" { powerpc*-*-darwin* } } */
> /* { dg-require-effective-target powerpc_p9vector_ok } */
> /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
> -/* { dg-options "-mcpu=power7 -mtune=power9 -O3" } */
> +/* { dg-options "-mcpu=power7 -mtune=power9 -O3 -dp" } */
>
> #define LARGE 0x12345
>
> @@ -12,7 +12,7 @@ int fusion_double_read (double *p){ return p[LARGE]; }
> void fusion_float_write (float *p, float f){ p[LARGE] = f; }
> void fusion_double_write (double *p, double d){ p[LARGE] = d; }
>
> -/* { dg-final { scan-assembler "load fusion, type SF" } } */
> -/* { dg-final { scan-assembler "load fusion, type DF" } } */
> -/* { dg-final { scan-assembler "store fusion, type SF" } } */
> -/* { dg-final { scan-assembler "store fusion, type DF" } } */
> +/* { dg-final { scan-assembler {fusion_vsx_[sd]i_sf_load} } } */
> +/* { dg-final { scan-assembler {fusion_vsx_[sd]i_df_load} } } */
> +/* { dg-final { scan-assembler {fusion_vsx_[sd]i_sf_store} } } */
> +/* { dg-final { scan-assembler {fusion_vsx_[sd]i_df_store} } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/fusion4.c b/gcc/testsuite/gcc.target/powerpc/fusion4.c
> index 2f3ad12..703c06c 100644
> --- a/gcc/testsuite/gcc.target/powerpc/fusion4.c
> +++ b/gcc/testsuite/gcc.target/powerpc/fusion4.c
> @@ -1,8 +1,7 @@
> -/* { dg-do compile { target { powerpc*-*-* && ilp32 } } } */
> /* { dg-skip-if "" { powerpc*-*-darwin* } } */
> /* { dg-require-effective-target powerpc_p9vector_ok } */
> /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
> -/* { dg-options "-mcpu=power7 -mtune=power9 -O3 -msoft-float" } */
> +/* { dg-options "-mcpu=power7 -mtune=power9 -O3 -msoft-float -dp" } */
>
> #define LARGE 0x12345
>
> @@ -10,4 +9,4 @@ float fusion_float_read (float *p){ return p[LARGE]; }
>
> void fusion_float_write (float *p, float f){ p[LARGE] = f; }
>
> -/* { dg-final { scan-assembler "store fusion, type SF" } } */
> +/* { dg-final { scan-assembler {fusion_gpr_[sd]i_sf_store} } } */
> --
> 1.8.3.1
>
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797