On Thu, Mar 31, 2022 at 7:51 AM liuhongt <hongtao....@intel.com> wrote:
>
> Since cfg is freed before machine_reorg, just do a rough calculation
> of the window according to the layout.
> Also according to an experiment on CLX, set window size to 64.
>
> Currently only handle V2DFmode load since it doesn't need any scratch
> registers, and it's sufficient to recover cray performance for -O2
> compared to GCC11.
>
> Bootstrap and regtest on x86_64-pc-linux-gnu{-m32,}.
> No impact for SPEC2017(same binary for both O2 and Ofast).
> Ok for trunk?
>
> gcc/ChangeLog:
>
> PR target/101908
> * config/i386/i386.cc (ix86_split_stlf_stall_load): New
> function
> (ix86_reorg): Call ix86_split_stlf_stall_load.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/i386/pr101908-1.c: New test.
> * gcc.target/i386/pr101908-2.c: New test.
> ---
> gcc/config/i386/i386.cc | 47 ++++++++++++++++++++++
> gcc/testsuite/gcc.target/i386/pr101908-1.c | 12 ++++++
> gcc/testsuite/gcc.target/i386/pr101908-2.c | 12 ++++++
> 3 files changed, 71 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/i386/pr101908-1.c
> create mode 100644 gcc/testsuite/gcc.target/i386/pr101908-2.c
>
> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
> index 5a561966eb4..f9169b04d43 100644
> --- a/gcc/config/i386/i386.cc
> +++ b/gcc/config/i386/i386.cc
> @@ -21933,7 +21933,53 @@ ix86_seh_fixup_eh_fallthru (void)
> emit_insn_after (gen_nops (const1_rtx), insn);
> }
> }
> +/* Split vector load from parm_decl to elemental loads to avoid STLF
> + stalls. */
> +static void
> +ix86_split_stlf_stall_load ()
> +{
> + basic_block bb;
> + unsigned window = 0;
> + FOR_EACH_BB_FN (bb, cfun)
> + {
> + rtx_insn *insn;
> + FOR_BB_INSNS (bb, insn)
> + {
> + if (!NONDEBUG_INSN_P (insn))
> + continue;
> + window++;
> + /* Insert 64 vaddps %xmm18, %xmm19, %xmm20(no dependence between
> each
> + other, just emulate for pipeline) before stalled load, stlf stall
> + case is as fast as no stall cases on CLX.
> + Since CFG is freed before machine_reorg, just do a rough
> + calculation of the window according to the layout. */
> + if (window > 64)
> + return;
I wonder if we should also return for any_uncondjump_p (insn)
(not sure if that captures returnjump_p), or maybe just explicitely
allow any_condjump_p and reject other PC setters.
Likewise we might want to stop at a LABEL that can be backwards reached.
I suppose people more familiar with cfgrtl can suggest something better.
> + rtx set = single_set (insn);
> + if (!set)
> + continue;
> + rtx src = SET_SRC (set);
> + if (!MEM_P (src)
> + /* Only handle V2DFmode load since it doesn't need any scratch
> + register. */
> + || GET_MODE (src) != E_V2DFmode
> + || !MEM_EXPR (src)
> + || TREE_CODE (get_base_address (MEM_EXPR (src))) != PARM_DECL)
I wonder if we have (easy) ways to detect whether XEXP (src, 0) is
frame/stack based
rather than requiring a MEM_EXPR. There is may_be_sp_based_p ()
exported from alias.c
for example, but I'm not sure whether that works after RA & frame elimination.
> + continue;
> +
> + rtx zero = CONST0_RTX (V2DFmode);
> + rtx dest = SET_DEST (set);
> + rtx m = adjust_address (src, DFmode, 0);
> + emit_insn_before (gen_sse2_loadlpd (dest, zero, m), insn);
Can SSE1 also do this?
> + m = adjust_address (src, DFmode, 8);
> + PATTERN (insn) = gen_sse2_loadhpd (dest, dest, m);
> + INSN_CODE (insn) = -1;
> + gcc_assert (recog_memoized (insn) != -1);
I think we want to dump something into dump_file when we split an insn here.
> + }
> + }
> +
> +}
> /* Implement machine specific optimizations. We implement padding of returns
> for K8 CPUs and pass to avoid 4 jumps in the single 16 byte window. */
> static void
> @@ -21948,6 +21994,7 @@ ix86_reorg (void)
>
> if (optimize && optimize_function_for_speed_p (cfun))
> {
> + ix86_split_stlf_stall_load ();
> if (TARGET_PAD_SHORT_FUNCTION)
> ix86_pad_short_function ();
btw. this function suggests we do have edges, so doing something "better"
than FOR_EACH_BB_FN, aka walking blocks in layout order, might be
possible after all. For example ix86_avoid_jump_mispredicts just walks
the function by looking at get_insns(), that might be more closely what
"as laid out" is.
> else if (TARGET_PAD_RETURNS)
> diff --git a/gcc/testsuite/gcc.target/i386/pr101908-1.c
> b/gcc/testsuite/gcc.target/i386/pr101908-1.c
> new file mode 100644
> index 00000000000..33d9684f0ad
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr101908-1.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -msse2 -mno-avx" } */
> +/* { dg-final { scan-assembler-not {(?n)movhpd[ \t]} } } */
> +
> +struct X { double x[2]; };
> +typedef double v2df __attribute__((vector_size(16)));
> +
> +v2df __attribute__((noipa))
> +foo (struct X* x, struct X* y)
> +{
> + return (v2df) {x->x[1], x->x[0] } + (v2df) { y->x[1], y->x[0] };
> +}
> diff --git a/gcc/testsuite/gcc.target/i386/pr101908-2.c
> b/gcc/testsuite/gcc.target/i386/pr101908-2.c
> new file mode 100644
> index 00000000000..45060b73c06
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr101908-2.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -msse2 -mno-avx" } */
> +/* { dg-final { scan-assembler-times {(?n)movhpd[ \t]+} "2" } } */
> +
> +struct X { double x[4]; };
> +typedef double v2df __attribute__((vector_size(16)));
> +
> +v2df __attribute__((noipa))
> +foo (struct X x, struct X y)
> +{
> + return (v2df) {x.x[1], x.x[0] } + (v2df) { y.x[1], y.x[0] };
> +}
> --
> 2.18.1
>
