| Message ID | e68958a9-d2d8-7922-e3c6-aa34b7582eeb@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Generate an lxvp instead of two adjacent lxv instructions | expand |
Hi! On Thu, Jul 08, 2021 at 05:01:05PM -0500, Peter Bergner wrote: > The MMA build built-ins currently use individual lxv instructions to > load up the registers of a __vector_pair or __vector_quad. If the > memory addresses of the built-in operands are to adjacent locations, > then we could use an lxvp in some cases to load up two registers at once. > The patch below adds support for checking whether memory addresses are > adjacent and emitting an lxvp instead of two lxv instructions. > > This passed bootstrap and regtesting on powerpc64le-linux with no regressions. > Ok for trunk? It needs testing on BE. > +static bool consecutive_mem_locations (rtx, rtx); Please don't; just move functions to somewhere earlier than where they are used. > @@ -16841,8 +16843,35 @@ rs6000_split_multireg_move (rtx dst, rtx src) > for (int i = 0; i < nvecs; i++) > { > int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; > - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); > - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); > + int index_next = WORDS_BIG_ENDIAN ? index + 1 : index - 1; What does index_next mean? The machine instructions do the same thing in any endianness. > + rtx dst_i; > + int regno = reg + i; > + > + /* If we are loading an even VSX register and our memory location > + is adjacent to the next register's memory location (if any), > + then we can load them both with one LXVP instruction. */ > + if ((regno & 1) == 0 > + && VSX_REGNO_P (regno) > + && MEM_P (XVECEXP (src, 0, index)) > + && MEM_P (XVECEXP (src, 0, index_next))) > + { > + rtx base = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index) > + : XVECEXP (src, 0, index_next); > + rtx next = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index_next) > + : XVECEXP (src, 0, index); Please get rid of index_next, if you still have to do different code for LE here -- it doesn't make the code any clearer (in fact I cannot follow it at all anymore :-( ) So this converts pairs of lxv to an lxvp in only a very limited case, right? Can we instead do it more generically? And what about stxvp? Segher
On 7/8/21 6:28 PM, Segher Boessenkool wrote: > It needs testing on BE. Will do. >> +static bool consecutive_mem_locations (rtx, rtx); > > Please don't; just move functions to somewhere earlier than where they > are used. Will do. >> @@ -16841,8 +16843,35 @@ rs6000_split_multireg_move (rtx dst, rtx src) >> for (int i = 0; i < nvecs; i++) >> { >> int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; >> - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); >> - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); >> + int index_next = WORDS_BIG_ENDIAN ? index + 1 : index - 1; > > What does index_next mean? The machine instructions do the same thing > in any endianness. Yeah, I'm bad at coming up with names! :-) So "index" is the index into XVECEXP (src, 0, ...) which is the operand that is to be assigned to regno. "index_next" is the index into XVECEXP (src, 0, ...) which is the operand to be assigned to regno + 1 (ie, the next register of the even/odd register pair). Whether the "next index" is index+1 or index-1 is dependent on LE versus BE. >> + /* If we are loading an even VSX register and our memory location >> + is adjacent to the next register's memory location (if any), >> + then we can load them both with one LXVP instruction. */ >> + if ((regno & 1) == 0 >> + && VSX_REGNO_P (regno) >> + && MEM_P (XVECEXP (src, 0, index)) >> + && MEM_P (XVECEXP (src, 0, index_next))) >> + { >> + rtx base = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index) >> + : XVECEXP (src, 0, index_next); >> + rtx next = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index_next) >> + : XVECEXP (src, 0, index); > > Please get rid of index_next, if you still have to do different code for > LE here -- it doesn't make the code any clearer (in fact I cannot follow > it at all anymore :-( ) We do need different code for LE versus BE. So you want something like if (WORDS_BIG_ENDIAN) {...} else {...} ...instead? I can try that to see if the code is easier to read. > So this converts pairs of lxv to an lxvp in only a very limited case, > right? Can we instead do it more generically? And what about stxvp? Doing it more generically is my next TODO and that will cover both lxvp and stxvp. My thought was to write a simple pass run at about the same time as our swap optimization pass to look for adjacent lxv's and stxv's and convert them into lxvp and stxvp. However, that won't catch the above case, since the assemble/build pattern is not split until very late, so we still want the above change. Also, given the new pass will be more complicated than the above code, it will be a GCC 12 only change. Let me make the changes you want and I'll repost with what I come up with. Peter
On Thu, Jul 08, 2021 at 08:26:45PM -0500, Peter Bergner wrote: > On 7/8/21 6:28 PM, Segher Boessenkool wrote: > >> int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; > >> - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); > >> - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); > >> + int index_next = WORDS_BIG_ENDIAN ? index + 1 : index - 1; > > > > What does index_next mean? The machine instructions do the same thing > > in any endianness. > > Yeah, I'm bad at coming up with names! :-) So "index" is the index > into XVECEXP (src, 0, ...) which is the operand that is to be assigned > to regno. "index_next" is the index into XVECEXP (src, 0, ...) which is > the operand to be assigned to regno + 1 (ie, the next register of the > even/odd register pair). Whether the "next index" is index+1 or index-1 > is dependent on LE versus BE. I would just call it "index1", or even "j" and "k" instead of "index" and "index_next" :-) "next" can put people on the wrong track (it did me :-) ) > >> + /* If we are loading an even VSX register and our memory location > >> + is adjacent to the next register's memory location (if any), > >> + then we can load them both with one LXVP instruction. */ > >> + if ((regno & 1) == 0 > >> + && VSX_REGNO_P (regno) > >> + && MEM_P (XVECEXP (src, 0, index)) > >> + && MEM_P (XVECEXP (src, 0, index_next))) > >> + { > >> + rtx base = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index) > >> + : XVECEXP (src, 0, index_next); > >> + rtx next = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index_next) > >> + : XVECEXP (src, 0, index); > > > > Please get rid of index_next, if you still have to do different code for > > LE here -- it doesn't make the code any clearer (in fact I cannot follow > > it at all anymore :-( ) > > We do need different code for LE versus BE. So you want something like > > if (WORDS_BIG_ENDIAN) {...} else {...} > > ...instead? I can try that to see if the code is easier to read. Yes exactly. It will more directly say what it does, and there is no "index_next" abstraction the reader has to absorb first. > > So this converts pairs of lxv to an lxvp in only a very limited case, > > right? Can we instead do it more generically? And what about stxvp? > > Doing it more generically is my next TODO and that will cover both > lxvp and stxvp. Ah cool :-) > My thought was to write a simple pass run at about > the same time as our swap optimization pass to look for adjacent > lxv's and stxv's and convert them into lxvp and stxvp. So, very early, as soon as DF is set up. Makes sense. > However, that > won't catch the above case, since the assemble/build pattern is not > split until very late, so we still want the above change. You probably should also have a peephole (whether you do it like here or not :-) ) > Also, given the new pass will be more complicated than the above code, > it will be a GCC 12 only change. /nod > Let me make the changes you want and I'll repost with what I come up with. Thanks! And thanks for the explanation. Segher
On 7/8/21 8:26 PM, Peter Bergner wrote: > We do need different code for LE versus BE. So you want something like > > if (WORDS_BIG_ENDIAN) {...} else {...} > > ...instead? I can try that to see if the code is easier to read. [snip] > Let me make the changes you want and I'll repost with what I come up with. Ok, I removed the consecutive_mem_locations() function from the previous patch and just call adjacent_mem_locations() directly now. I also moved rs6000_split_multireg_move() to later in the file to fix the declaration issue. However, since rs6000_split_multireg_move() is where the new code was added to emit the lxvp's, it can be hard to see what I changed because of the move. I'll note that all of my changes are restrictd to within the if (GET_CODE (src) == UNSPEC) { gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE); ... } ...code section. Does this look better? I'm currently running bootstraps and regtests on LE and BE. Peter gcc/ * config/rs6000/rs6000.c (rs6000_split_multireg_move): Move to later in the file. Handle MMA build built-ins with operands in adjacent memory locations. (adjacent_mem_locations): Test that MEM1 and MEM2 are MEMs. Return the lower addressed memory rtx, if any. (power6_sched_reorder2): Update for adjacent_mem_locations change. gcc/testsuite/ * gcc.target/powerpc/mma-builtin-9.c: New test. diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 9a5db63d0ef..8edf7a4a81c 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -16690,382 +16690,6 @@ rs6000_expand_atomic_op (enum rtx_code code, rtx mem, rtx val, emit_move_insn (orig_after, after); } -/* Emit instructions to move SRC to DST. Called by splitters for - multi-register moves. It will emit at most one instruction for - each register that is accessed; that is, it won't emit li/lis pairs - (or equivalent for 64-bit code). One of SRC or DST must be a hard - register. */ - -void -rs6000_split_multireg_move (rtx dst, rtx src) -{ - /* The register number of the first register being moved. */ - int reg; - /* The mode that is to be moved. */ - machine_mode mode; - /* The mode that the move is being done in, and its size. */ - machine_mode reg_mode; - int reg_mode_size; - /* The number of registers that will be moved. */ - int nregs; - - reg = REG_P (dst) ? REGNO (dst) : REGNO (src); - mode = GET_MODE (dst); - nregs = hard_regno_nregs (reg, mode); - - /* If we have a vector quad register for MMA, and this is a load or store, - see if we can use vector paired load/stores. */ - if (mode == XOmode && TARGET_MMA - && (MEM_P (dst) || MEM_P (src))) - { - reg_mode = OOmode; - nregs /= 2; - } - /* If we have a vector pair/quad mode, split it into two/four separate - vectors. */ - else if (mode == OOmode || mode == XOmode) - reg_mode = V1TImode; - else if (FP_REGNO_P (reg)) - reg_mode = DECIMAL_FLOAT_MODE_P (mode) ? DDmode : - (TARGET_HARD_FLOAT ? DFmode : SFmode); - else if (ALTIVEC_REGNO_P (reg)) - reg_mode = V16QImode; - else - reg_mode = word_mode; - reg_mode_size = GET_MODE_SIZE (reg_mode); - - gcc_assert (reg_mode_size * nregs == GET_MODE_SIZE (mode)); - - /* TDmode residing in FP registers is special, since the ISA requires that - the lower-numbered word of a register pair is always the most significant - word, even in little-endian mode. This does not match the usual subreg - semantics, so we cannnot use simplify_gen_subreg in those cases. Access - the appropriate constituent registers "by hand" in little-endian mode. - - Note we do not need to check for destructive overlap here since TDmode - can only reside in even/odd register pairs. */ - if (FP_REGNO_P (reg) && DECIMAL_FLOAT_MODE_P (mode) && !BYTES_BIG_ENDIAN) - { - rtx p_src, p_dst; - int i; - - for (i = 0; i < nregs; i++) - { - if (REG_P (src) && FP_REGNO_P (REGNO (src))) - p_src = gen_rtx_REG (reg_mode, REGNO (src) + nregs - 1 - i); - else - p_src = simplify_gen_subreg (reg_mode, src, mode, - i * reg_mode_size); - - if (REG_P (dst) && FP_REGNO_P (REGNO (dst))) - p_dst = gen_rtx_REG (reg_mode, REGNO (dst) + nregs - 1 - i); - else - p_dst = simplify_gen_subreg (reg_mode, dst, mode, - i * reg_mode_size); - - emit_insn (gen_rtx_SET (p_dst, p_src)); - } - - return; - } - - /* The __vector_pair and __vector_quad modes are multi-register - modes, so if we have to load or store the registers, we have to be - careful to properly swap them if we're in little endian mode - below. This means the last register gets the first memory - location. We also need to be careful of using the right register - numbers if we are splitting XO to OO. */ - if (mode == OOmode || mode == XOmode) - { - nregs = hard_regno_nregs (reg, mode); - int reg_mode_nregs = hard_regno_nregs (reg, reg_mode); - if (MEM_P (dst)) - { - unsigned offset = 0; - unsigned size = GET_MODE_SIZE (reg_mode); - - /* If we are reading an accumulator register, we have to - deprime it before we can access it. */ - if (TARGET_MMA - && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) - emit_insn (gen_mma_xxmfacc (src, src)); - - for (int i = 0; i < nregs; i += reg_mode_nregs) - { - unsigned subreg = - (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); - rtx dst2 = adjust_address (dst, reg_mode, offset); - rtx src2 = gen_rtx_REG (reg_mode, reg + subreg); - offset += size; - emit_insn (gen_rtx_SET (dst2, src2)); - } - - return; - } - - if (MEM_P (src)) - { - unsigned offset = 0; - unsigned size = GET_MODE_SIZE (reg_mode); - - for (int i = 0; i < nregs; i += reg_mode_nregs) - { - unsigned subreg = - (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); - rtx dst2 = gen_rtx_REG (reg_mode, reg + subreg); - rtx src2 = adjust_address (src, reg_mode, offset); - offset += size; - emit_insn (gen_rtx_SET (dst2, src2)); - } - - /* If we are writing an accumulator register, we have to - prime it after we've written it. */ - if (TARGET_MMA - && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) - emit_insn (gen_mma_xxmtacc (dst, dst)); - - return; - } - - if (GET_CODE (src) == UNSPEC) - { - gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE); - gcc_assert (REG_P (dst)); - if (GET_MODE (src) == XOmode) - gcc_assert (FP_REGNO_P (REGNO (dst))); - if (GET_MODE (src) == OOmode) - gcc_assert (VSX_REGNO_P (REGNO (dst))); - - reg_mode = GET_MODE (XVECEXP (src, 0, 0)); - int nvecs = XVECLEN (src, 0); - for (int i = 0; i < nvecs; i++) - { - int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); - } - - /* We are writing an accumulator register, so we have to - prime it after we've written it. */ - if (GET_MODE (src) == XOmode) - emit_insn (gen_mma_xxmtacc (dst, dst)); - - return; - } - - /* Register -> register moves can use common code. */ - } - - if (REG_P (src) && REG_P (dst) && (REGNO (src) < REGNO (dst))) - { - /* If we are reading an accumulator register, we have to - deprime it before we can access it. */ - if (TARGET_MMA - && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) - emit_insn (gen_mma_xxmfacc (src, src)); - - /* Move register range backwards, if we might have destructive - overlap. */ - int i; - /* XO/OO are opaque so cannot use subregs. */ - if (mode == OOmode || mode == XOmode ) - { - for (i = nregs - 1; i >= 0; i--) - { - rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + i); - rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + i); - emit_insn (gen_rtx_SET (dst_i, src_i)); - } - } - else - { - for (i = nregs - 1; i >= 0; i--) - emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, - i * reg_mode_size), - simplify_gen_subreg (reg_mode, src, mode, - i * reg_mode_size))); - } - - /* If we are writing an accumulator register, we have to - prime it after we've written it. */ - if (TARGET_MMA - && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) - emit_insn (gen_mma_xxmtacc (dst, dst)); - } - else - { - int i; - int j = -1; - bool used_update = false; - rtx restore_basereg = NULL_RTX; - - if (MEM_P (src) && INT_REGNO_P (reg)) - { - rtx breg; - - if (GET_CODE (XEXP (src, 0)) == PRE_INC - || GET_CODE (XEXP (src, 0)) == PRE_DEC) - { - rtx delta_rtx; - breg = XEXP (XEXP (src, 0), 0); - delta_rtx = (GET_CODE (XEXP (src, 0)) == PRE_INC - ? GEN_INT (GET_MODE_SIZE (GET_MODE (src))) - : GEN_INT (-GET_MODE_SIZE (GET_MODE (src)))); - emit_insn (gen_add3_insn (breg, breg, delta_rtx)); - src = replace_equiv_address (src, breg); - } - else if (! rs6000_offsettable_memref_p (src, reg_mode, true)) - { - if (GET_CODE (XEXP (src, 0)) == PRE_MODIFY) - { - rtx basereg = XEXP (XEXP (src, 0), 0); - if (TARGET_UPDATE) - { - rtx ndst = simplify_gen_subreg (reg_mode, dst, mode, 0); - emit_insn (gen_rtx_SET (ndst, - gen_rtx_MEM (reg_mode, - XEXP (src, 0)))); - used_update = true; - } - else - emit_insn (gen_rtx_SET (basereg, - XEXP (XEXP (src, 0), 1))); - src = replace_equiv_address (src, basereg); - } - else - { - rtx basereg = gen_rtx_REG (Pmode, reg); - emit_insn (gen_rtx_SET (basereg, XEXP (src, 0))); - src = replace_equiv_address (src, basereg); - } - } - - breg = XEXP (src, 0); - if (GET_CODE (breg) == PLUS || GET_CODE (breg) == LO_SUM) - breg = XEXP (breg, 0); - - /* If the base register we are using to address memory is - also a destination reg, then change that register last. */ - if (REG_P (breg) - && REGNO (breg) >= REGNO (dst) - && REGNO (breg) < REGNO (dst) + nregs) - j = REGNO (breg) - REGNO (dst); - } - else if (MEM_P (dst) && INT_REGNO_P (reg)) - { - rtx breg; - - if (GET_CODE (XEXP (dst, 0)) == PRE_INC - || GET_CODE (XEXP (dst, 0)) == PRE_DEC) - { - rtx delta_rtx; - breg = XEXP (XEXP (dst, 0), 0); - delta_rtx = (GET_CODE (XEXP (dst, 0)) == PRE_INC - ? GEN_INT (GET_MODE_SIZE (GET_MODE (dst))) - : GEN_INT (-GET_MODE_SIZE (GET_MODE (dst)))); - - /* We have to update the breg before doing the store. - Use store with update, if available. */ - - if (TARGET_UPDATE) - { - rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); - emit_insn (TARGET_32BIT - ? (TARGET_POWERPC64 - ? gen_movdi_si_update (breg, breg, delta_rtx, nsrc) - : gen_movsi_si_update (breg, breg, delta_rtx, nsrc)) - : gen_movdi_di_update (breg, breg, delta_rtx, nsrc)); - used_update = true; - } - else - emit_insn (gen_add3_insn (breg, breg, delta_rtx)); - dst = replace_equiv_address (dst, breg); - } - else if (!rs6000_offsettable_memref_p (dst, reg_mode, true) - && GET_CODE (XEXP (dst, 0)) != LO_SUM) - { - if (GET_CODE (XEXP (dst, 0)) == PRE_MODIFY) - { - rtx basereg = XEXP (XEXP (dst, 0), 0); - if (TARGET_UPDATE) - { - rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); - emit_insn (gen_rtx_SET (gen_rtx_MEM (reg_mode, - XEXP (dst, 0)), - nsrc)); - used_update = true; - } - else - emit_insn (gen_rtx_SET (basereg, - XEXP (XEXP (dst, 0), 1))); - dst = replace_equiv_address (dst, basereg); - } - else - { - rtx basereg = XEXP (XEXP (dst, 0), 0); - rtx offsetreg = XEXP (XEXP (dst, 0), 1); - gcc_assert (GET_CODE (XEXP (dst, 0)) == PLUS - && REG_P (basereg) - && REG_P (offsetreg) - && REGNO (basereg) != REGNO (offsetreg)); - if (REGNO (basereg) == 0) - { - rtx tmp = offsetreg; - offsetreg = basereg; - basereg = tmp; - } - emit_insn (gen_add3_insn (basereg, basereg, offsetreg)); - restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg); - dst = replace_equiv_address (dst, basereg); - } - } - else if (GET_CODE (XEXP (dst, 0)) != LO_SUM) - gcc_assert (rs6000_offsettable_memref_p (dst, reg_mode, true)); - } - - /* If we are reading an accumulator register, we have to - deprime it before we can access it. */ - if (TARGET_MMA && REG_P (src) - && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) - emit_insn (gen_mma_xxmfacc (src, src)); - - for (i = 0; i < nregs; i++) - { - /* Calculate index to next subword. */ - ++j; - if (j == nregs) - j = 0; - - /* If compiler already emitted move of first word by - store with update, no need to do anything. */ - if (j == 0 && used_update) - continue; - - /* XO/OO are opaque so cannot use subregs. */ - if (mode == OOmode || mode == XOmode ) - { - rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + j); - rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + j); - emit_insn (gen_rtx_SET (dst_i, src_i)); - } - else - emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, - j * reg_mode_size), - simplify_gen_subreg (reg_mode, src, mode, - j * reg_mode_size))); - } - - /* If we are writing an accumulator register, we have to - prime it after we've written it. */ - if (TARGET_MMA && REG_P (dst) - && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) - emit_insn (gen_mma_xxmtacc (dst, dst)); - - if (restore_basereg != NULL_RTX) - emit_insn (restore_basereg); - } -} - static GTY(()) alias_set_type TOC_alias_set = -1; alias_set_type @@ -18427,23 +18051,29 @@ get_memref_parts (rtx mem, rtx *base, HOST_WIDE_INT *offset, return true; } -/* The function returns true if the target storage location of - mem1 is adjacent to the target storage location of mem2 */ -/* Return 1 if memory locations are adjacent. */ +/* If the target storage locations of arguments MEM1 and MEM2 are + adjacent, then return the argument that has the lower address. + Otherwise, return NULL_RTX. */ -static bool +static rtx adjacent_mem_locations (rtx mem1, rtx mem2) { rtx reg1, reg2; HOST_WIDE_INT off1, size1, off2, size2; - if (get_memref_parts (mem1, ®1, &off1, &size1) - && get_memref_parts (mem2, ®2, &off2, &size2)) - return ((REGNO (reg1) == REGNO (reg2)) - && ((off1 + size1 == off2) - || (off2 + size2 == off1))); + if (MEM_P (mem1) + && MEM_P (mem2) + && get_memref_parts (mem1, ®1, &off1, &size1) + && get_memref_parts (mem2, ®2, &off2, &size2) + && REGNO (reg1) == REGNO (reg2)) + { + if (off1 + size1 == off2) + return mem1; + else if (off2 + size2 == off1) + return mem2; + } - return false; + return NULL_RTX; } /* This function returns true if it can be determined that the two MEM @@ -19009,7 +18639,7 @@ power6_sched_reorder2 (rtx_insn **ready, int lastpos) first_store_pos = pos; if (is_store_insn (last_scheduled_insn, &str_mem2) - && adjacent_mem_locations (str_mem, str_mem2)) + && adjacent_mem_locations (str_mem, str_mem2) != NULL_RTX) { /* Found an adjacent store. Move it to the head of the ready list, and adjust it's priority so that it is @@ -26982,6 +26612,416 @@ rs6000_split_logical (rtx operands[3], return; } +/* Emit instructions to move SRC to DST. Called by splitters for + multi-register moves. It will emit at most one instruction for + each register that is accessed; that is, it won't emit li/lis pairs + (or equivalent for 64-bit code). One of SRC or DST must be a hard + register. */ + +void +rs6000_split_multireg_move (rtx dst, rtx src) +{ + /* The register number of the first register being moved. */ + int reg; + /* The mode that is to be moved. */ + machine_mode mode; + /* The mode that the move is being done in, and its size. */ + machine_mode reg_mode; + int reg_mode_size; + /* The number of registers that will be moved. */ + int nregs; + + reg = REG_P (dst) ? REGNO (dst) : REGNO (src); + mode = GET_MODE (dst); + nregs = hard_regno_nregs (reg, mode); + + /* If we have a vector quad register for MMA, and this is a load or store, + see if we can use vector paired load/stores. */ + if (mode == XOmode && TARGET_MMA + && (MEM_P (dst) || MEM_P (src))) + { + reg_mode = OOmode; + nregs /= 2; + } + /* If we have a vector pair/quad mode, split it into two/four separate + vectors. */ + else if (mode == OOmode || mode == XOmode) + reg_mode = V1TImode; + else if (FP_REGNO_P (reg)) + reg_mode = DECIMAL_FLOAT_MODE_P (mode) ? DDmode : + (TARGET_HARD_FLOAT ? DFmode : SFmode); + else if (ALTIVEC_REGNO_P (reg)) + reg_mode = V16QImode; + else + reg_mode = word_mode; + reg_mode_size = GET_MODE_SIZE (reg_mode); + + gcc_assert (reg_mode_size * nregs == GET_MODE_SIZE (mode)); + + /* TDmode residing in FP registers is special, since the ISA requires that + the lower-numbered word of a register pair is always the most significant + word, even in little-endian mode. This does not match the usual subreg + semantics, so we cannnot use simplify_gen_subreg in those cases. Access + the appropriate constituent registers "by hand" in little-endian mode. + + Note we do not need to check for destructive overlap here since TDmode + can only reside in even/odd register pairs. */ + if (FP_REGNO_P (reg) && DECIMAL_FLOAT_MODE_P (mode) && !BYTES_BIG_ENDIAN) + { + rtx p_src, p_dst; + int i; + + for (i = 0; i < nregs; i++) + { + if (REG_P (src) && FP_REGNO_P (REGNO (src))) + p_src = gen_rtx_REG (reg_mode, REGNO (src) + nregs - 1 - i); + else + p_src = simplify_gen_subreg (reg_mode, src, mode, + i * reg_mode_size); + + if (REG_P (dst) && FP_REGNO_P (REGNO (dst))) + p_dst = gen_rtx_REG (reg_mode, REGNO (dst) + nregs - 1 - i); + else + p_dst = simplify_gen_subreg (reg_mode, dst, mode, + i * reg_mode_size); + + emit_insn (gen_rtx_SET (p_dst, p_src)); + } + + return; + } + + /* The __vector_pair and __vector_quad modes are multi-register + modes, so if we have to load or store the registers, we have to be + careful to properly swap them if we're in little endian mode + below. This means the last register gets the first memory + location. We also need to be careful of using the right register + numbers if we are splitting XO to OO. */ + if (mode == OOmode || mode == XOmode) + { + nregs = hard_regno_nregs (reg, mode); + int reg_mode_nregs = hard_regno_nregs (reg, reg_mode); + if (MEM_P (dst)) + { + unsigned offset = 0; + unsigned size = GET_MODE_SIZE (reg_mode); + + /* If we are reading an accumulator register, we have to + deprime it before we can access it. */ + if (TARGET_MMA + && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) + emit_insn (gen_mma_xxmfacc (src, src)); + + for (int i = 0; i < nregs; i += reg_mode_nregs) + { + unsigned subreg = + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); + rtx dst2 = adjust_address (dst, reg_mode, offset); + rtx src2 = gen_rtx_REG (reg_mode, reg + subreg); + offset += size; + emit_insn (gen_rtx_SET (dst2, src2)); + } + + return; + } + + if (MEM_P (src)) + { + unsigned offset = 0; + unsigned size = GET_MODE_SIZE (reg_mode); + + for (int i = 0; i < nregs; i += reg_mode_nregs) + { + unsigned subreg = + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); + rtx dst2 = gen_rtx_REG (reg_mode, reg + subreg); + rtx src2 = adjust_address (src, reg_mode, offset); + offset += size; + emit_insn (gen_rtx_SET (dst2, src2)); + } + + /* If we are writing an accumulator register, we have to + prime it after we've written it. */ + if (TARGET_MMA + && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) + emit_insn (gen_mma_xxmtacc (dst, dst)); + + return; + } + + if (GET_CODE (src) == UNSPEC) + { + gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE); + gcc_assert (REG_P (dst)); + if (GET_MODE (src) == XOmode) + gcc_assert (FP_REGNO_P (REGNO (dst))); + if (GET_MODE (src) == OOmode) + gcc_assert (VSX_REGNO_P (REGNO (dst))); + + int nvecs = XVECLEN (src, 0); + for (int i = 0; i < nvecs; i++) + { + rtx opnd; + int regno = reg + i; + + if (WORDS_BIG_ENDIAN) + opnd = XVECEXP (src, 0, i); + else + opnd = XVECEXP (src, 0, nvecs - i - 1); + + /* If we are loading an even VSX register and the memory location + is adjacent to the next register's memory location (if any), + then we can load them both with one LXVP instruction. */ + if ((regno & 1) == 0) + { + if (WORDS_BIG_ENDIAN) + { + rtx opnd2 = XVECEXP (src, 0, i + 1); + if (adjacent_mem_locations (opnd, opnd2) == opnd) + { + opnd = adjust_address (opnd, OOmode, 0); + /* Skip the next register, since we're going to + load it together with this register. */ + i++; + } + } + else + { + rtx opnd2 = XVECEXP (src, 0, nvecs - i - 2); + if (adjacent_mem_locations (opnd2, opnd) == opnd2) + { + opnd = adjust_address (opnd2, OOmode, 0); + /* Skip the next register, since we're going to + load it together with this register. */ + i++; + } + } + } + + rtx dst_i = gen_rtx_REG (GET_MODE (opnd), regno); + emit_insn (gen_rtx_SET (dst_i, opnd)); + } + + /* We are writing an accumulator register, so we have to + prime it after we've written it. */ + if (GET_MODE (src) == XOmode) + emit_insn (gen_mma_xxmtacc (dst, dst)); + + return; + } + + /* Register -> register moves can use common code. */ + } + + if (REG_P (src) && REG_P (dst) && (REGNO (src) < REGNO (dst))) + { + /* If we are reading an accumulator register, we have to + deprime it before we can access it. */ + if (TARGET_MMA + && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) + emit_insn (gen_mma_xxmfacc (src, src)); + + /* Move register range backwards, if we might have destructive + overlap. */ + int i; + /* XO/OO are opaque so cannot use subregs. */ + if (mode == OOmode || mode == XOmode ) + { + for (i = nregs - 1; i >= 0; i--) + { + rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + i); + rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + i); + emit_insn (gen_rtx_SET (dst_i, src_i)); + } + } + else + { + for (i = nregs - 1; i >= 0; i--) + emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, + i * reg_mode_size), + simplify_gen_subreg (reg_mode, src, mode, + i * reg_mode_size))); + } + + /* If we are writing an accumulator register, we have to + prime it after we've written it. */ + if (TARGET_MMA + && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) + emit_insn (gen_mma_xxmtacc (dst, dst)); + } + else + { + int i; + int j = -1; + bool used_update = false; + rtx restore_basereg = NULL_RTX; + + if (MEM_P (src) && INT_REGNO_P (reg)) + { + rtx breg; + + if (GET_CODE (XEXP (src, 0)) == PRE_INC + || GET_CODE (XEXP (src, 0)) == PRE_DEC) + { + rtx delta_rtx; + breg = XEXP (XEXP (src, 0), 0); + delta_rtx = (GET_CODE (XEXP (src, 0)) == PRE_INC + ? GEN_INT (GET_MODE_SIZE (GET_MODE (src))) + : GEN_INT (-GET_MODE_SIZE (GET_MODE (src)))); + emit_insn (gen_add3_insn (breg, breg, delta_rtx)); + src = replace_equiv_address (src, breg); + } + else if (! rs6000_offsettable_memref_p (src, reg_mode, true)) + { + if (GET_CODE (XEXP (src, 0)) == PRE_MODIFY) + { + rtx basereg = XEXP (XEXP (src, 0), 0); + if (TARGET_UPDATE) + { + rtx ndst = simplify_gen_subreg (reg_mode, dst, mode, 0); + emit_insn (gen_rtx_SET (ndst, + gen_rtx_MEM (reg_mode, + XEXP (src, 0)))); + used_update = true; + } + else + emit_insn (gen_rtx_SET (basereg, + XEXP (XEXP (src, 0), 1))); + src = replace_equiv_address (src, basereg); + } + else + { + rtx basereg = gen_rtx_REG (Pmode, reg); + emit_insn (gen_rtx_SET (basereg, XEXP (src, 0))); + src = replace_equiv_address (src, basereg); + } + } + + breg = XEXP (src, 0); + if (GET_CODE (breg) == PLUS || GET_CODE (breg) == LO_SUM) + breg = XEXP (breg, 0); + + /* If the base register we are using to address memory is + also a destination reg, then change that register last. */ + if (REG_P (breg) + && REGNO (breg) >= REGNO (dst) + && REGNO (breg) < REGNO (dst) + nregs) + j = REGNO (breg) - REGNO (dst); + } + else if (MEM_P (dst) && INT_REGNO_P (reg)) + { + rtx breg; + + if (GET_CODE (XEXP (dst, 0)) == PRE_INC + || GET_CODE (XEXP (dst, 0)) == PRE_DEC) + { + rtx delta_rtx; + breg = XEXP (XEXP (dst, 0), 0); + delta_rtx = (GET_CODE (XEXP (dst, 0)) == PRE_INC + ? GEN_INT (GET_MODE_SIZE (GET_MODE (dst))) + : GEN_INT (-GET_MODE_SIZE (GET_MODE (dst)))); + + /* We have to update the breg before doing the store. + Use store with update, if available. */ + + if (TARGET_UPDATE) + { + rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); + emit_insn (TARGET_32BIT + ? (TARGET_POWERPC64 + ? gen_movdi_si_update (breg, breg, delta_rtx, nsrc) + : gen_movsi_si_update (breg, breg, delta_rtx, nsrc)) + : gen_movdi_di_update (breg, breg, delta_rtx, nsrc)); + used_update = true; + } + else + emit_insn (gen_add3_insn (breg, breg, delta_rtx)); + dst = replace_equiv_address (dst, breg); + } + else if (!rs6000_offsettable_memref_p (dst, reg_mode, true) + && GET_CODE (XEXP (dst, 0)) != LO_SUM) + { + if (GET_CODE (XEXP (dst, 0)) == PRE_MODIFY) + { + rtx basereg = XEXP (XEXP (dst, 0), 0); + if (TARGET_UPDATE) + { + rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); + emit_insn (gen_rtx_SET (gen_rtx_MEM (reg_mode, + XEXP (dst, 0)), + nsrc)); + used_update = true; + } + else + emit_insn (gen_rtx_SET (basereg, + XEXP (XEXP (dst, 0), 1))); + dst = replace_equiv_address (dst, basereg); + } + else + { + rtx basereg = XEXP (XEXP (dst, 0), 0); + rtx offsetreg = XEXP (XEXP (dst, 0), 1); + gcc_assert (GET_CODE (XEXP (dst, 0)) == PLUS + && REG_P (basereg) + && REG_P (offsetreg) + && REGNO (basereg) != REGNO (offsetreg)); + if (REGNO (basereg) == 0) + { + rtx tmp = offsetreg; + offsetreg = basereg; + basereg = tmp; + } + emit_insn (gen_add3_insn (basereg, basereg, offsetreg)); + restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg); + dst = replace_equiv_address (dst, basereg); + } + } + else if (GET_CODE (XEXP (dst, 0)) != LO_SUM) + gcc_assert (rs6000_offsettable_memref_p (dst, reg_mode, true)); + } + + /* If we are reading an accumulator register, we have to + deprime it before we can access it. */ + if (TARGET_MMA && REG_P (src) + && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) + emit_insn (gen_mma_xxmfacc (src, src)); + + for (i = 0; i < nregs; i++) + { + /* Calculate index to next subword. */ + ++j; + if (j == nregs) + j = 0; + + /* If compiler already emitted move of first word by + store with update, no need to do anything. */ + if (j == 0 && used_update) + continue; + + /* XO/OO are opaque so cannot use subregs. */ + if (mode == OOmode || mode == XOmode ) + { + rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + j); + rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + j); + emit_insn (gen_rtx_SET (dst_i, src_i)); + } + else + emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, + j * reg_mode_size), + simplify_gen_subreg (reg_mode, src, mode, + j * reg_mode_size))); + } + + /* If we are writing an accumulator register, we have to + prime it after we've written it. */ + if (TARGET_MMA && REG_P (dst) + && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) + emit_insn (gen_mma_xxmtacc (dst, dst)); + + if (restore_basereg != NULL_RTX) + emit_insn (restore_basereg); + } +} /* Return true if the peephole2 can combine a load involving a combination of an addis instruction and a load with an offset that can be fused together on diff --git a/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c b/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c new file mode 100644 index 00000000000..397d0f1db35 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target power10_ok } */ +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */ + +typedef unsigned char vec_t __attribute__((vector_size(16))); + +void +foo (__vector_pair *dst, vec_t *src) +{ + __vector_pair pair; + /* Adjacent loads should be combined into one lxvp instruction. */ + __builtin_vsx_build_pair (&pair, src[0], src[1]); + *dst = pair; +} + +void +bar (__vector_quad *dst, vec_t *src) +{ + __vector_quad quad; + /* Adjacent loads should be combined into two lxvp instructions. */ + __builtin_mma_build_acc (&quad, src[0], src[1], src[2], src[3]); + *dst = quad; +} + +/* { dg-final { scan-assembler-not {\mlxv\M} } } */ +/* { dg-final { scan-assembler-not {\mstxv\M} } } */ +/* { dg-final { scan-assembler-times {\mlxvp\M} 3 } } */ +/* { dg-final { scan-assembler-times {\mstxvp\M} 3 } } */
On 7/9/21 6:14 PM, Peter Bergner wrote: > ...code section. Does this look better? I'm currently running bootstraps > and regtests on LE and BE. Bootstrap and regtesting on both LE and BE showed no regressions. Peter
On Fri, Jul 09, 2021 at 06:14:49PM -0500, Peter Bergner wrote: > Ok, I removed the consecutive_mem_locations() function from the previous > patch and just call adjacent_mem_locations() directly now. I also moved > rs6000_split_multireg_move() to later in the file to fix the declaration > issue. However, since rs6000_split_multireg_move() is where the new code > was added to emit the lxvp's, it can be hard to see what I changed because > of the move. I'll note that all of my changes are restrictd to within the > > if (GET_CODE (src) == UNSPEC) > { > gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE); > ... > } > > ...code section. Does this look better? I'm currently running bootstraps > and regtests on LE and BE. It is very hard to see the differences now. Don't fold the changes into one patch, just have the code movement in a separate trivial patch, and then the actual changes as a separate patch? That way it is much easier to review :-) > + unsigned subreg = > + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); This is not new code, but it caught my eye, so just for the record: the "=" should start a new line: unsigned subreg = WORDS_BIG_ENDIAN ? i : (nregs - reg_mode_nregs - i); (and don't put parens around random words please :-) ). > + int nvecs = XVECLEN (src, 0); > + for (int i = 0; i < nvecs; i++) > + { > + rtx opnd; Just "op" (and "op2") please? If you use long names you might as well just spell "operand" :-) > + if (WORDS_BIG_ENDIAN) > + opnd = XVECEXP (src, 0, i); > + else > + opnd = XVECEXP (src, 0, nvecs - i - 1); Put this together with the case below as well? Probably keep the WORDS_BIG_ENDIAN test as the outer "if"? > + /* If we are loading an even VSX register and the memory location > + is adjacent to the next register's memory location (if any), > + then we can load them both with one LXVP instruction. */ > + if ((regno & 1) == 0) > + { > + if (WORDS_BIG_ENDIAN) > + { > + rtx opnd2 = XVECEXP (src, 0, i + 1); > + if (adjacent_mem_locations (opnd, opnd2) == opnd) > + { > + opnd = adjust_address (opnd, OOmode, 0); > + /* Skip the next register, since we're going to > + load it together with this register. */ > + i++; > + } > + } > + else > + { > + rtx opnd2 = XVECEXP (src, 0, nvecs - i - 2); > + if (adjacent_mem_locations (opnd2, opnd) == opnd2) > + { > + opnd = adjust_address (opnd2, OOmode, 0); > + /* Skip the next register, since we're going to > + load it together with this register. */ > + i++; > + } > + } > + } I think it is fine now, but please factor the patch and repost. Thanks! Segher
On 7/10/21 7:39 PM, segher@gate.crashing.org wrote: > It is very hard to see the differences now. Don't fold the changes into > one patch, just have the code movement in a separate trivial patch, and > then the actual changes as a separate patch? That way it is much easier > to review :-) Ok, I split the patch into 2 patches. The one here is simply the move. The second patch in a different reply will handle your other comments. Peter rs6000: Move rs6000_split_multireg_move to later in file An upcoming change to rs6000_split_multireg_move requires it to be moved later in the file to fix a declaration issue. gcc/ * config/rs6000/rs6000.c (rs6000_split_multireg_move): Move to later in the file. --- gcc/config/rs6000/rs6000.c | 751 ++++++++++++++++++------------------- 1 file changed, 375 insertions(+), 376 deletions(-) diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 9a5db63d0ef..ae11b8d52cb 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -16690,382 +16690,6 @@ rs6000_expand_atomic_op (enum rtx_code code, rtx mem, rtx val, emit_move_insn (orig_after, after); } -/* Emit instructions to move SRC to DST. Called by splitters for - multi-register moves. It will emit at most one instruction for - each register that is accessed; that is, it won't emit li/lis pairs - (or equivalent for 64-bit code). One of SRC or DST must be a hard - register. */ - -void -rs6000_split_multireg_move (rtx dst, rtx src) -{ - /* The register number of the first register being moved. */ - int reg; - /* The mode that is to be moved. */ - machine_mode mode; - /* The mode that the move is being done in, and its size. */ - machine_mode reg_mode; - int reg_mode_size; - /* The number of registers that will be moved. */ - int nregs; - - reg = REG_P (dst) ? REGNO (dst) : REGNO (src); - mode = GET_MODE (dst); - nregs = hard_regno_nregs (reg, mode); - - /* If we have a vector quad register for MMA, and this is a load or store, - see if we can use vector paired load/stores. */ - if (mode == XOmode && TARGET_MMA - && (MEM_P (dst) || MEM_P (src))) - { - reg_mode = OOmode; - nregs /= 2; - } - /* If we have a vector pair/quad mode, split it into two/four separate - vectors. */ - else if (mode == OOmode || mode == XOmode) - reg_mode = V1TImode; - else if (FP_REGNO_P (reg)) - reg_mode = DECIMAL_FLOAT_MODE_P (mode) ? DDmode : - (TARGET_HARD_FLOAT ? DFmode : SFmode); - else if (ALTIVEC_REGNO_P (reg)) - reg_mode = V16QImode; - else - reg_mode = word_mode; - reg_mode_size = GET_MODE_SIZE (reg_mode); - - gcc_assert (reg_mode_size * nregs == GET_MODE_SIZE (mode)); - - /* TDmode residing in FP registers is special, since the ISA requires that - the lower-numbered word of a register pair is always the most significant - word, even in little-endian mode. This does not match the usual subreg - semantics, so we cannnot use simplify_gen_subreg in those cases. Access - the appropriate constituent registers "by hand" in little-endian mode. - - Note we do not need to check for destructive overlap here since TDmode - can only reside in even/odd register pairs. */ - if (FP_REGNO_P (reg) && DECIMAL_FLOAT_MODE_P (mode) && !BYTES_BIG_ENDIAN) - { - rtx p_src, p_dst; - int i; - - for (i = 0; i < nregs; i++) - { - if (REG_P (src) && FP_REGNO_P (REGNO (src))) - p_src = gen_rtx_REG (reg_mode, REGNO (src) + nregs - 1 - i); - else - p_src = simplify_gen_subreg (reg_mode, src, mode, - i * reg_mode_size); - - if (REG_P (dst) && FP_REGNO_P (REGNO (dst))) - p_dst = gen_rtx_REG (reg_mode, REGNO (dst) + nregs - 1 - i); - else - p_dst = simplify_gen_subreg (reg_mode, dst, mode, - i * reg_mode_size); - - emit_insn (gen_rtx_SET (p_dst, p_src)); - } - - return; - } - - /* The __vector_pair and __vector_quad modes are multi-register - modes, so if we have to load or store the registers, we have to be - careful to properly swap them if we're in little endian mode - below. This means the last register gets the first memory - location. We also need to be careful of using the right register - numbers if we are splitting XO to OO. */ - if (mode == OOmode || mode == XOmode) - { - nregs = hard_regno_nregs (reg, mode); - int reg_mode_nregs = hard_regno_nregs (reg, reg_mode); - if (MEM_P (dst)) - { - unsigned offset = 0; - unsigned size = GET_MODE_SIZE (reg_mode); - - /* If we are reading an accumulator register, we have to - deprime it before we can access it. */ - if (TARGET_MMA - && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) - emit_insn (gen_mma_xxmfacc (src, src)); - - for (int i = 0; i < nregs; i += reg_mode_nregs) - { - unsigned subreg = - (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); - rtx dst2 = adjust_address (dst, reg_mode, offset); - rtx src2 = gen_rtx_REG (reg_mode, reg + subreg); - offset += size; - emit_insn (gen_rtx_SET (dst2, src2)); - } - - return; - } - - if (MEM_P (src)) - { - unsigned offset = 0; - unsigned size = GET_MODE_SIZE (reg_mode); - - for (int i = 0; i < nregs; i += reg_mode_nregs) - { - unsigned subreg = - (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); - rtx dst2 = gen_rtx_REG (reg_mode, reg + subreg); - rtx src2 = adjust_address (src, reg_mode, offset); - offset += size; - emit_insn (gen_rtx_SET (dst2, src2)); - } - - /* If we are writing an accumulator register, we have to - prime it after we've written it. */ - if (TARGET_MMA - && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) - emit_insn (gen_mma_xxmtacc (dst, dst)); - - return; - } - - if (GET_CODE (src) == UNSPEC) - { - gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE); - gcc_assert (REG_P (dst)); - if (GET_MODE (src) == XOmode) - gcc_assert (FP_REGNO_P (REGNO (dst))); - if (GET_MODE (src) == OOmode) - gcc_assert (VSX_REGNO_P (REGNO (dst))); - - reg_mode = GET_MODE (XVECEXP (src, 0, 0)); - int nvecs = XVECLEN (src, 0); - for (int i = 0; i < nvecs; i++) - { - int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); - } - - /* We are writing an accumulator register, so we have to - prime it after we've written it. */ - if (GET_MODE (src) == XOmode) - emit_insn (gen_mma_xxmtacc (dst, dst)); - - return; - } - - /* Register -> register moves can use common code. */ - } - - if (REG_P (src) && REG_P (dst) && (REGNO (src) < REGNO (dst))) - { - /* If we are reading an accumulator register, we have to - deprime it before we can access it. */ - if (TARGET_MMA - && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) - emit_insn (gen_mma_xxmfacc (src, src)); - - /* Move register range backwards, if we might have destructive - overlap. */ - int i; - /* XO/OO are opaque so cannot use subregs. */ - if (mode == OOmode || mode == XOmode ) - { - for (i = nregs - 1; i >= 0; i--) - { - rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + i); - rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + i); - emit_insn (gen_rtx_SET (dst_i, src_i)); - } - } - else - { - for (i = nregs - 1; i >= 0; i--) - emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, - i * reg_mode_size), - simplify_gen_subreg (reg_mode, src, mode, - i * reg_mode_size))); - } - - /* If we are writing an accumulator register, we have to - prime it after we've written it. */ - if (TARGET_MMA - && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) - emit_insn (gen_mma_xxmtacc (dst, dst)); - } - else - { - int i; - int j = -1; - bool used_update = false; - rtx restore_basereg = NULL_RTX; - - if (MEM_P (src) && INT_REGNO_P (reg)) - { - rtx breg; - - if (GET_CODE (XEXP (src, 0)) == PRE_INC - || GET_CODE (XEXP (src, 0)) == PRE_DEC) - { - rtx delta_rtx; - breg = XEXP (XEXP (src, 0), 0); - delta_rtx = (GET_CODE (XEXP (src, 0)) == PRE_INC - ? GEN_INT (GET_MODE_SIZE (GET_MODE (src))) - : GEN_INT (-GET_MODE_SIZE (GET_MODE (src)))); - emit_insn (gen_add3_insn (breg, breg, delta_rtx)); - src = replace_equiv_address (src, breg); - } - else if (! rs6000_offsettable_memref_p (src, reg_mode, true)) - { - if (GET_CODE (XEXP (src, 0)) == PRE_MODIFY) - { - rtx basereg = XEXP (XEXP (src, 0), 0); - if (TARGET_UPDATE) - { - rtx ndst = simplify_gen_subreg (reg_mode, dst, mode, 0); - emit_insn (gen_rtx_SET (ndst, - gen_rtx_MEM (reg_mode, - XEXP (src, 0)))); - used_update = true; - } - else - emit_insn (gen_rtx_SET (basereg, - XEXP (XEXP (src, 0), 1))); - src = replace_equiv_address (src, basereg); - } - else - { - rtx basereg = gen_rtx_REG (Pmode, reg); - emit_insn (gen_rtx_SET (basereg, XEXP (src, 0))); - src = replace_equiv_address (src, basereg); - } - } - - breg = XEXP (src, 0); - if (GET_CODE (breg) == PLUS || GET_CODE (breg) == LO_SUM) - breg = XEXP (breg, 0); - - /* If the base register we are using to address memory is - also a destination reg, then change that register last. */ - if (REG_P (breg) - && REGNO (breg) >= REGNO (dst) - && REGNO (breg) < REGNO (dst) + nregs) - j = REGNO (breg) - REGNO (dst); - } - else if (MEM_P (dst) && INT_REGNO_P (reg)) - { - rtx breg; - - if (GET_CODE (XEXP (dst, 0)) == PRE_INC - || GET_CODE (XEXP (dst, 0)) == PRE_DEC) - { - rtx delta_rtx; - breg = XEXP (XEXP (dst, 0), 0); - delta_rtx = (GET_CODE (XEXP (dst, 0)) == PRE_INC - ? GEN_INT (GET_MODE_SIZE (GET_MODE (dst))) - : GEN_INT (-GET_MODE_SIZE (GET_MODE (dst)))); - - /* We have to update the breg before doing the store. - Use store with update, if available. */ - - if (TARGET_UPDATE) - { - rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); - emit_insn (TARGET_32BIT - ? (TARGET_POWERPC64 - ? gen_movdi_si_update (breg, breg, delta_rtx, nsrc) - : gen_movsi_si_update (breg, breg, delta_rtx, nsrc)) - : gen_movdi_di_update (breg, breg, delta_rtx, nsrc)); - used_update = true; - } - else - emit_insn (gen_add3_insn (breg, breg, delta_rtx)); - dst = replace_equiv_address (dst, breg); - } - else if (!rs6000_offsettable_memref_p (dst, reg_mode, true) - && GET_CODE (XEXP (dst, 0)) != LO_SUM) - { - if (GET_CODE (XEXP (dst, 0)) == PRE_MODIFY) - { - rtx basereg = XEXP (XEXP (dst, 0), 0); - if (TARGET_UPDATE) - { - rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); - emit_insn (gen_rtx_SET (gen_rtx_MEM (reg_mode, - XEXP (dst, 0)), - nsrc)); - used_update = true; - } - else - emit_insn (gen_rtx_SET (basereg, - XEXP (XEXP (dst, 0), 1))); - dst = replace_equiv_address (dst, basereg); - } - else - { - rtx basereg = XEXP (XEXP (dst, 0), 0); - rtx offsetreg = XEXP (XEXP (dst, 0), 1); - gcc_assert (GET_CODE (XEXP (dst, 0)) == PLUS - && REG_P (basereg) - && REG_P (offsetreg) - && REGNO (basereg) != REGNO (offsetreg)); - if (REGNO (basereg) == 0) - { - rtx tmp = offsetreg; - offsetreg = basereg; - basereg = tmp; - } - emit_insn (gen_add3_insn (basereg, basereg, offsetreg)); - restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg); - dst = replace_equiv_address (dst, basereg); - } - } - else if (GET_CODE (XEXP (dst, 0)) != LO_SUM) - gcc_assert (rs6000_offsettable_memref_p (dst, reg_mode, true)); - } - - /* If we are reading an accumulator register, we have to - deprime it before we can access it. */ - if (TARGET_MMA && REG_P (src) - && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) - emit_insn (gen_mma_xxmfacc (src, src)); - - for (i = 0; i < nregs; i++) - { - /* Calculate index to next subword. */ - ++j; - if (j == nregs) - j = 0; - - /* If compiler already emitted move of first word by - store with update, no need to do anything. */ - if (j == 0 && used_update) - continue; - - /* XO/OO are opaque so cannot use subregs. */ - if (mode == OOmode || mode == XOmode ) - { - rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + j); - rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + j); - emit_insn (gen_rtx_SET (dst_i, src_i)); - } - else - emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, - j * reg_mode_size), - simplify_gen_subreg (reg_mode, src, mode, - j * reg_mode_size))); - } - - /* If we are writing an accumulator register, we have to - prime it after we've written it. */ - if (TARGET_MMA && REG_P (dst) - && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) - emit_insn (gen_mma_xxmtacc (dst, dst)); - - if (restore_basereg != NULL_RTX) - emit_insn (restore_basereg); - } -} - static GTY(()) alias_set_type TOC_alias_set = -1; alias_set_type @@ -26982,6 +26606,381 @@ rs6000_split_logical (rtx operands[3], return; } +/* Emit instructions to move SRC to DST. Called by splitters for + multi-register moves. It will emit at most one instruction for + each register that is accessed; that is, it won't emit li/lis pairs + (or equivalent for 64-bit code). One of SRC or DST must be a hard + register. */ + +void +rs6000_split_multireg_move (rtx dst, rtx src) +{ + /* The register number of the first register being moved. */ + int reg; + /* The mode that is to be moved. */ + machine_mode mode; + /* The mode that the move is being done in, and its size. */ + machine_mode reg_mode; + int reg_mode_size; + /* The number of registers that will be moved. */ + int nregs; + + reg = REG_P (dst) ? REGNO (dst) : REGNO (src); + mode = GET_MODE (dst); + nregs = hard_regno_nregs (reg, mode); + + /* If we have a vector quad register for MMA, and this is a load or store, + see if we can use vector paired load/stores. */ + if (mode == XOmode && TARGET_MMA + && (MEM_P (dst) || MEM_P (src))) + { + reg_mode = OOmode; + nregs /= 2; + } + /* If we have a vector pair/quad mode, split it into two/four separate + vectors. */ + else if (mode == OOmode || mode == XOmode) + reg_mode = V1TImode; + else if (FP_REGNO_P (reg)) + reg_mode = DECIMAL_FLOAT_MODE_P (mode) ? DDmode : + (TARGET_HARD_FLOAT ? DFmode : SFmode); + else if (ALTIVEC_REGNO_P (reg)) + reg_mode = V16QImode; + else + reg_mode = word_mode; + reg_mode_size = GET_MODE_SIZE (reg_mode); + + gcc_assert (reg_mode_size * nregs == GET_MODE_SIZE (mode)); + + /* TDmode residing in FP registers is special, since the ISA requires that + the lower-numbered word of a register pair is always the most significant + word, even in little-endian mode. This does not match the usual subreg + semantics, so we cannnot use simplify_gen_subreg in those cases. Access + the appropriate constituent registers "by hand" in little-endian mode. + + Note we do not need to check for destructive overlap here since TDmode + can only reside in even/odd register pairs. */ + if (FP_REGNO_P (reg) && DECIMAL_FLOAT_MODE_P (mode) && !BYTES_BIG_ENDIAN) + { + rtx p_src, p_dst; + int i; + + for (i = 0; i < nregs; i++) + { + if (REG_P (src) && FP_REGNO_P (REGNO (src))) + p_src = gen_rtx_REG (reg_mode, REGNO (src) + nregs - 1 - i); + else + p_src = simplify_gen_subreg (reg_mode, src, mode, + i * reg_mode_size); + + if (REG_P (dst) && FP_REGNO_P (REGNO (dst))) + p_dst = gen_rtx_REG (reg_mode, REGNO (dst) + nregs - 1 - i); + else + p_dst = simplify_gen_subreg (reg_mode, dst, mode, + i * reg_mode_size); + + emit_insn (gen_rtx_SET (p_dst, p_src)); + } + + return; + } + + /* The __vector_pair and __vector_quad modes are multi-register + modes, so if we have to load or store the registers, we have to be + careful to properly swap them if we're in little endian mode + below. This means the last register gets the first memory + location. We also need to be careful of using the right register + numbers if we are splitting XO to OO. */ + if (mode == OOmode || mode == XOmode) + { + nregs = hard_regno_nregs (reg, mode); + int reg_mode_nregs = hard_regno_nregs (reg, reg_mode); + if (MEM_P (dst)) + { + unsigned offset = 0; + unsigned size = GET_MODE_SIZE (reg_mode); + + /* If we are reading an accumulator register, we have to + deprime it before we can access it. */ + if (TARGET_MMA + && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) + emit_insn (gen_mma_xxmfacc (src, src)); + + for (int i = 0; i < nregs; i += reg_mode_nregs) + { + unsigned subreg = + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); + rtx dst2 = adjust_address (dst, reg_mode, offset); + rtx src2 = gen_rtx_REG (reg_mode, reg + subreg); + offset += size; + emit_insn (gen_rtx_SET (dst2, src2)); + } + + return; + } + + if (MEM_P (src)) + { + unsigned offset = 0; + unsigned size = GET_MODE_SIZE (reg_mode); + + for (int i = 0; i < nregs; i += reg_mode_nregs) + { + unsigned subreg = + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); + rtx dst2 = gen_rtx_REG (reg_mode, reg + subreg); + rtx src2 = adjust_address (src, reg_mode, offset); + offset += size; + emit_insn (gen_rtx_SET (dst2, src2)); + } + + /* If we are writing an accumulator register, we have to + prime it after we've written it. */ + if (TARGET_MMA + && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) + emit_insn (gen_mma_xxmtacc (dst, dst)); + + return; + } + + if (GET_CODE (src) == UNSPEC) + { + gcc_assert (XINT (src, 1) == UNSPEC_MMA_ASSEMBLE); + gcc_assert (REG_P (dst)); + if (GET_MODE (src) == XOmode) + gcc_assert (FP_REGNO_P (REGNO (dst))); + if (GET_MODE (src) == OOmode) + gcc_assert (VSX_REGNO_P (REGNO (dst))); + + reg_mode = GET_MODE (XVECEXP (src, 0, 0)); + int nvecs = XVECLEN (src, 0); + for (int i = 0; i < nvecs; i++) + { + int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; + rtx dst_i = gen_rtx_REG (reg_mode, reg + index); + emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); + } + + /* We are writing an accumulator register, so we have to + prime it after we've written it. */ + if (GET_MODE (src) == XOmode) + emit_insn (gen_mma_xxmtacc (dst, dst)); + + return; + } + + /* Register -> register moves can use common code. */ + } + + if (REG_P (src) && REG_P (dst) && (REGNO (src) < REGNO (dst))) + { + /* If we are reading an accumulator register, we have to + deprime it before we can access it. */ + if (TARGET_MMA + && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) + emit_insn (gen_mma_xxmfacc (src, src)); + + /* Move register range backwards, if we might have destructive + overlap. */ + int i; + /* XO/OO are opaque so cannot use subregs. */ + if (mode == OOmode || mode == XOmode ) + { + for (i = nregs - 1; i >= 0; i--) + { + rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + i); + rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + i); + emit_insn (gen_rtx_SET (dst_i, src_i)); + } + } + else + { + for (i = nregs - 1; i >= 0; i--) + emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, + i * reg_mode_size), + simplify_gen_subreg (reg_mode, src, mode, + i * reg_mode_size))); + } + + /* If we are writing an accumulator register, we have to + prime it after we've written it. */ + if (TARGET_MMA + && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) + emit_insn (gen_mma_xxmtacc (dst, dst)); + } + else + { + int i; + int j = -1; + bool used_update = false; + rtx restore_basereg = NULL_RTX; + + if (MEM_P (src) && INT_REGNO_P (reg)) + { + rtx breg; + + if (GET_CODE (XEXP (src, 0)) == PRE_INC + || GET_CODE (XEXP (src, 0)) == PRE_DEC) + { + rtx delta_rtx; + breg = XEXP (XEXP (src, 0), 0); + delta_rtx = (GET_CODE (XEXP (src, 0)) == PRE_INC + ? GEN_INT (GET_MODE_SIZE (GET_MODE (src))) + : GEN_INT (-GET_MODE_SIZE (GET_MODE (src)))); + emit_insn (gen_add3_insn (breg, breg, delta_rtx)); + src = replace_equiv_address (src, breg); + } + else if (! rs6000_offsettable_memref_p (src, reg_mode, true)) + { + if (GET_CODE (XEXP (src, 0)) == PRE_MODIFY) + { + rtx basereg = XEXP (XEXP (src, 0), 0); + if (TARGET_UPDATE) + { + rtx ndst = simplify_gen_subreg (reg_mode, dst, mode, 0); + emit_insn (gen_rtx_SET (ndst, + gen_rtx_MEM (reg_mode, + XEXP (src, 0)))); + used_update = true; + } + else + emit_insn (gen_rtx_SET (basereg, + XEXP (XEXP (src, 0), 1))); + src = replace_equiv_address (src, basereg); + } + else + { + rtx basereg = gen_rtx_REG (Pmode, reg); + emit_insn (gen_rtx_SET (basereg, XEXP (src, 0))); + src = replace_equiv_address (src, basereg); + } + } + + breg = XEXP (src, 0); + if (GET_CODE (breg) == PLUS || GET_CODE (breg) == LO_SUM) + breg = XEXP (breg, 0); + + /* If the base register we are using to address memory is + also a destination reg, then change that register last. */ + if (REG_P (breg) + && REGNO (breg) >= REGNO (dst) + && REGNO (breg) < REGNO (dst) + nregs) + j = REGNO (breg) - REGNO (dst); + } + else if (MEM_P (dst) && INT_REGNO_P (reg)) + { + rtx breg; + + if (GET_CODE (XEXP (dst, 0)) == PRE_INC + || GET_CODE (XEXP (dst, 0)) == PRE_DEC) + { + rtx delta_rtx; + breg = XEXP (XEXP (dst, 0), 0); + delta_rtx = (GET_CODE (XEXP (dst, 0)) == PRE_INC + ? GEN_INT (GET_MODE_SIZE (GET_MODE (dst))) + : GEN_INT (-GET_MODE_SIZE (GET_MODE (dst)))); + + /* We have to update the breg before doing the store. + Use store with update, if available. */ + + if (TARGET_UPDATE) + { + rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); + emit_insn (TARGET_32BIT + ? (TARGET_POWERPC64 + ? gen_movdi_si_update (breg, breg, delta_rtx, nsrc) + : gen_movsi_si_update (breg, breg, delta_rtx, nsrc)) + : gen_movdi_di_update (breg, breg, delta_rtx, nsrc)); + used_update = true; + } + else + emit_insn (gen_add3_insn (breg, breg, delta_rtx)); + dst = replace_equiv_address (dst, breg); + } + else if (!rs6000_offsettable_memref_p (dst, reg_mode, true) + && GET_CODE (XEXP (dst, 0)) != LO_SUM) + { + if (GET_CODE (XEXP (dst, 0)) == PRE_MODIFY) + { + rtx basereg = XEXP (XEXP (dst, 0), 0); + if (TARGET_UPDATE) + { + rtx nsrc = simplify_gen_subreg (reg_mode, src, mode, 0); + emit_insn (gen_rtx_SET (gen_rtx_MEM (reg_mode, + XEXP (dst, 0)), + nsrc)); + used_update = true; + } + else + emit_insn (gen_rtx_SET (basereg, + XEXP (XEXP (dst, 0), 1))); + dst = replace_equiv_address (dst, basereg); + } + else + { + rtx basereg = XEXP (XEXP (dst, 0), 0); + rtx offsetreg = XEXP (XEXP (dst, 0), 1); + gcc_assert (GET_CODE (XEXP (dst, 0)) == PLUS + && REG_P (basereg) + && REG_P (offsetreg) + && REGNO (basereg) != REGNO (offsetreg)); + if (REGNO (basereg) == 0) + { + rtx tmp = offsetreg; + offsetreg = basereg; + basereg = tmp; + } + emit_insn (gen_add3_insn (basereg, basereg, offsetreg)); + restore_basereg = gen_sub3_insn (basereg, basereg, offsetreg); + dst = replace_equiv_address (dst, basereg); + } + } + else if (GET_CODE (XEXP (dst, 0)) != LO_SUM) + gcc_assert (rs6000_offsettable_memref_p (dst, reg_mode, true)); + } + + /* If we are reading an accumulator register, we have to + deprime it before we can access it. */ + if (TARGET_MMA && REG_P (src) + && GET_MODE (src) == XOmode && FP_REGNO_P (REGNO (src))) + emit_insn (gen_mma_xxmfacc (src, src)); + + for (i = 0; i < nregs; i++) + { + /* Calculate index to next subword. */ + ++j; + if (j == nregs) + j = 0; + + /* If compiler already emitted move of first word by + store with update, no need to do anything. */ + if (j == 0 && used_update) + continue; + + /* XO/OO are opaque so cannot use subregs. */ + if (mode == OOmode || mode == XOmode ) + { + rtx dst_i = gen_rtx_REG (reg_mode, REGNO (dst) + j); + rtx src_i = gen_rtx_REG (reg_mode, REGNO (src) + j); + emit_insn (gen_rtx_SET (dst_i, src_i)); + } + else + emit_insn (gen_rtx_SET (simplify_gen_subreg (reg_mode, dst, mode, + j * reg_mode_size), + simplify_gen_subreg (reg_mode, src, mode, + j * reg_mode_size))); + } + + /* If we are writing an accumulator register, we have to + prime it after we've written it. */ + if (TARGET_MMA && REG_P (dst) + && GET_MODE (dst) == XOmode && FP_REGNO_P (REGNO (dst))) + emit_insn (gen_mma_xxmtacc (dst, dst)); + + if (restore_basereg != NULL_RTX) + emit_insn (restore_basereg); + } +} /* Return true if the peephole2 can combine a load involving a combination of an addis instruction and a load with an offset that can be fused together on
...and patch 2: On 7/10/21 7:39 PM, segher@gate.crashing.org wrote: >> + unsigned subreg = >> + (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); > > This is not new code, but it caught my eye, so just for the record: the > "=" should start a new line: > unsigned subreg > = WORDS_BIG_ENDIAN ? i : (nregs - reg_mode_nregs - i); > (and don't put parens around random words please :-) ). Fixed. >> + int nvecs = XVECLEN (src, 0); >> + for (int i = 0; i < nvecs; i++) >> + { >> + rtx opnd; > > Just "op" (and "op2") please? If you use long names you might as well > just spell "operand" :-) Done. >> + if (WORDS_BIG_ENDIAN) >> + opnd = XVECEXP (src, 0, i); >> + else >> + opnd = XVECEXP (src, 0, nvecs - i - 1); > > Put this together with the case below as well? Probably keep the > WORDS_BIG_ENDIAN test as the outer "if"? Ok, reworked a little bit. I'm currently bootstrapping and regtesting these two patches and will report back. Better now? Peter rs6000: Generate an lxvp instead of two adjacent lxv instructions The MMA build built-ins currently use individual lxv instructions to load up the registers of a __vector_pair or __vector_quad. If the memory addresses of the built-in operands are to adjacent locations, then we can use an lxvp in some cases to load up two registers at once. The patch below adds support for checking whether memory addresses are adjacent and emitting an lxvp instead of two lxv instructions. gcc/ * config/rs6000/rs6000.c (adjacent_mem_locations): Return the lower addressed memory rtx, if any. (power6_sched_reorder2): Update for adjacent_mem_locations change. (rs6000_split_multireg_move): Fix code formatting. Handle MMA build built-ins with operands in adjacent memory locations. gcc/testsuite/ * gcc.target/powerpc/mma-builtin-9.c: New test. --- gcc/config/rs6000/rs6000.c | 84 ++++++++++++++----- .../gcc.target/powerpc/mma-builtin-9.c | 28 +++++++ 2 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index ae11b8d52cb..5fed3bc3ac1 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -18051,23 +18051,29 @@ get_memref_parts (rtx mem, rtx *base, HOST_WIDE_INT *offset, return true; } -/* The function returns true if the target storage location of - mem1 is adjacent to the target storage location of mem2 */ -/* Return 1 if memory locations are adjacent. */ +/* If the target storage locations of arguments MEM1 and MEM2 are + adjacent, then return the argument that has the lower address. + Otherwise, return NULL_RTX. */ -static bool +static rtx adjacent_mem_locations (rtx mem1, rtx mem2) { rtx reg1, reg2; HOST_WIDE_INT off1, size1, off2, size2; - if (get_memref_parts (mem1, ®1, &off1, &size1) - && get_memref_parts (mem2, ®2, &off2, &size2)) - return ((REGNO (reg1) == REGNO (reg2)) - && ((off1 + size1 == off2) - || (off2 + size2 == off1))); + if (MEM_P (mem1) + && MEM_P (mem2) + && get_memref_parts (mem1, ®1, &off1, &size1) + && get_memref_parts (mem2, ®2, &off2, &size2) + && REGNO (reg1) == REGNO (reg2)) + { + if (off1 + size1 == off2) + return mem1; + else if (off2 + size2 == off1) + return mem2; + } - return false; + return NULL_RTX; } /* This function returns true if it can be determined that the two MEM @@ -18633,7 +18639,7 @@ power6_sched_reorder2 (rtx_insn **ready, int lastpos) first_store_pos = pos; if (is_store_insn (last_scheduled_insn, &str_mem2) - && adjacent_mem_locations (str_mem, str_mem2)) + && adjacent_mem_locations (str_mem, str_mem2) != NULL_RTX) { /* Found an adjacent store. Move it to the head of the ready list, and adjust it's priority so that it is @@ -26708,8 +26714,8 @@ rs6000_split_multireg_move (rtx dst, rtx src) for (int i = 0; i < nregs; i += reg_mode_nregs) { - unsigned subreg = - (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); + unsigned subreg + = WORDS_BIG_ENDIAN ? i : (nregs - reg_mode_nregs - i); rtx dst2 = adjust_address (dst, reg_mode, offset); rtx src2 = gen_rtx_REG (reg_mode, reg + subreg); offset += size; @@ -26726,8 +26732,8 @@ rs6000_split_multireg_move (rtx dst, rtx src) for (int i = 0; i < nregs; i += reg_mode_nregs) { - unsigned subreg = - (WORDS_BIG_ENDIAN) ? i : (nregs - reg_mode_nregs - i); + unsigned subreg + = WORDS_BIG_ENDIAN ? i : (nregs - reg_mode_nregs - i); rtx dst2 = gen_rtx_REG (reg_mode, reg + subreg); rtx src2 = adjust_address (src, reg_mode, offset); offset += size; @@ -26752,13 +26758,53 @@ rs6000_split_multireg_move (rtx dst, rtx src) if (GET_MODE (src) == OOmode) gcc_assert (VSX_REGNO_P (REGNO (dst))); - reg_mode = GET_MODE (XVECEXP (src, 0, 0)); int nvecs = XVECLEN (src, 0); for (int i = 0; i < nvecs; i++) { - int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); + rtx op; + int regno = reg + i; + + if (WORDS_BIG_ENDIAN) + { + op = XVECEXP (src, 0, i); + + /* If we are loading an even VSX register and the memory location + is adjacent to the next register's memory location (if any), + then we can load them both with one LXVP instruction. */ + if ((regno & 1) == 0) + { + rtx op2 = XVECEXP (src, 0, i + 1); + if (adjacent_mem_locations (op, op2) == op) + { + op = adjust_address (op, OOmode, 0); + /* Skip the next register, since we're going to + load it together with this register. */ + i++; + } + } + } + else + { + op = XVECEXP (src, 0, nvecs - i - 1); + + /* If we are loading an even VSX register and the memory location + is adjacent to the next register's memory location (if any), + then we can load them both with one LXVP instruction. */ + if ((regno & 1) == 0) + { + rtx op2 = XVECEXP (src, 0, nvecs - i - 2); + if (adjacent_mem_locations (op2, op) == op2) + { + op = adjust_address (op2, OOmode, 0); + /* Skip the next register, since we're going to + load it together with this register. */ + i++; + } + } + } + + rtx dst_i = gen_rtx_REG (GET_MODE (op), regno); + emit_insn (gen_rtx_SET (dst_i, op)); } /* We are writing an accumulator register, so we have to diff --git a/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c b/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c new file mode 100644 index 00000000000..397d0f1db35 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target power10_ok } */ +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */ + +typedef unsigned char vec_t __attribute__((vector_size(16))); + +void +foo (__vector_pair *dst, vec_t *src) +{ + __vector_pair pair; + /* Adjacent loads should be combined into one lxvp instruction. */ + __builtin_vsx_build_pair (&pair, src[0], src[1]); + *dst = pair; +} + +void +bar (__vector_quad *dst, vec_t *src) +{ + __vector_quad quad; + /* Adjacent loads should be combined into two lxvp instructions. */ + __builtin_mma_build_acc (&quad, src[0], src[1], src[2], src[3]); + *dst = quad; +} + +/* { dg-final { scan-assembler-not {\mlxv\M} } } */ +/* { dg-final { scan-assembler-not {\mstxv\M} } } */ +/* { dg-final { scan-assembler-times {\mlxvp\M} 3 } } */ +/* { dg-final { scan-assembler-times {\mstxvp\M} 3 } } */
On 7/13/21 12:14 PM, Peter Bergner wrote: > ...and patch 2: [snip] > I'm currently bootstrapping and regtesting these two patches and > will report back. Better now? Ok, this along with the previous move patch bootstrapped and regtested with no regressions on powerpc64le-linux. Peter
On Tue, Jul 13, 2021 at 12:09:07PM -0500, Peter Bergner wrote: > On 7/10/21 7:39 PM, segher@gate.crashing.org wrote: > > It is very hard to see the differences now. Don't fold the changes into > > one patch, just have the code movement in a separate trivial patch, and > > then the actual changes as a separate patch? That way it is much easier > > to review :-) > > Ok, I split the patch into 2 patches. The one here is simply the move. This one is obviously okay for trunk. Thanks! Segher
Hi! On Tue, Jul 13, 2021 at 12:14:22PM -0500, Peter Bergner wrote: > +/* If the target storage locations of arguments MEM1 and MEM2 are > + adjacent, then return the argument that has the lower address. > + Otherwise, return NULL_RTX. */ > > -static bool > +static rtx > adjacent_mem_locations (rtx mem1, rtx mem2) Note that the function name now nly makes much sense if you use the return value as a boolean (zero for false, non-zero for true). > @@ -18633,7 +18639,7 @@ power6_sched_reorder2 (rtx_insn **ready, int lastpos) > first_store_pos = pos; > > if (is_store_insn (last_scheduled_insn, &str_mem2) > - && adjacent_mem_locations (str_mem, str_mem2)) > + && adjacent_mem_locations (str_mem, str_mem2) != NULL_RTX) ... so don't change this? Or write != 0 != 0 != 0, if one time is good, three times must be better! :-) > @@ -26752,13 +26758,53 @@ rs6000_split_multireg_move (rtx dst, rtx src) > if (GET_MODE (src) == OOmode) > gcc_assert (VSX_REGNO_P (REGNO (dst))); > > - reg_mode = GET_MODE (XVECEXP (src, 0, 0)); > int nvecs = XVECLEN (src, 0); > for (int i = 0; i < nvecs; i++) > { > - int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; > - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); > - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); > + rtx op; > + int regno = reg + i; > + > + if (WORDS_BIG_ENDIAN) > + { > + op = XVECEXP (src, 0, i); > + > + /* If we are loading an even VSX register and the memory location > + is adjacent to the next register's memory location (if any), > + then we can load them both with one LXVP instruction. */ > + if ((regno & 1) == 0) > + { > + rtx op2 = XVECEXP (src, 0, i + 1); > + if (adjacent_mem_locations (op, op2) == op) > + { > + op = adjust_address (op, OOmode, 0); > + /* Skip the next register, since we're going to > + load it together with this register. */ > + i++; > + } > + } > + } > + else > + { > + op = XVECEXP (src, 0, nvecs - i - 1); > + > + /* If we are loading an even VSX register and the memory location > + is adjacent to the next register's memory location (if any), > + then we can load them both with one LXVP instruction. */ > + if ((regno & 1) == 0) > + { > + rtx op2 = XVECEXP (src, 0, nvecs - i - 2); > + if (adjacent_mem_locations (op2, op) == op2) > + { > + op = adjust_address (op2, OOmode, 0); > + /* Skip the next register, since we're going to > + load it together with this register. */ > + i++; > + } > + } > + } > + > + rtx dst_i = gen_rtx_REG (GET_MODE (op), regno); > + emit_insn (gen_rtx_SET (dst_i, op)); > } So we are sure we have a hard register here, and it is a VSX register. Okay. Factoring this code would not hurt ;-) Okay for trunk. Thanks! Segher
On 7/13/21 5:59 PM, Segher Boessenkool wrote: >> - && adjacent_mem_locations (str_mem, str_mem2)) >> + && adjacent_mem_locations (str_mem, str_mem2) != NULL_RTX) > > ... so don't change this? Or write != 0 != 0 != 0, if one time is good, > three times must be better! :-) Ok, consider those two changes reverted and will leave the code as is. > So we are sure we have a hard register here, and it is a VSX register. > Okay. Factoring this code would not hurt ;-) Yes, we have asserts above that ensure we have regs and that they are of the correct type (ie, FP or VSX register) depending on the mode. I'll make the change above and rebuild just to be safe and then commit. Thanks. Peter
On 7/14/21 4:12 PM, Peter Bergner wrote:
> I'll make the change above and rebuild just to be safe and then commit.
Regtesting was clean as expected, so I pushed the commit to trunk. Thanks.
Is this ok for backporting to GCC 11 after a day or two on trunk?
Given GCC 10 doesn't have the opaque mode changes, I don't want this in GCC 10.
Peter
On Thu, Jul 15, 2021 at 09:15:55AM -0500, Peter Bergner wrote: > On 7/14/21 4:12 PM, Peter Bergner wrote: > > I'll make the change above and rebuild just to be safe and then commit. > > Regtesting was clean as expected, so I pushed the commit to trunk. Thanks. > Is this ok for backporting to GCC 11 after a day or two on trunk? If it is tested well enough, yes. There are many things that can break in this code, so I am not very comfortable with backporting it so close to a release, but if it is important, we can take that risk. Thanks, Segher
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 9a5db63d0ef..de36c5ecd91 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -293,6 +293,8 @@ bool cpu_builtin_p = false; don't link in rs6000-c.c, so we can't call it directly. */ void (*rs6000_target_modify_macros_ptr) (bool, HOST_WIDE_INT, HOST_WIDE_INT); +static bool consecutive_mem_locations (rtx, rtx); + /* Simplfy register classes into simpler classifications. We assume GPR_REG_TYPE - FPR_REG_TYPE are ordered so that we can use a simple range check for standard register classes (gpr/floating/altivec/vsx) and @@ -16841,8 +16843,35 @@ rs6000_split_multireg_move (rtx dst, rtx src) for (int i = 0; i < nvecs; i++) { int index = WORDS_BIG_ENDIAN ? i : nvecs - 1 - i; - rtx dst_i = gen_rtx_REG (reg_mode, reg + index); - emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, i))); + int index_next = WORDS_BIG_ENDIAN ? index + 1 : index - 1; + rtx dst_i; + int regno = reg + i; + + /* If we are loading an even VSX register and our memory location + is adjacent to the next register's memory location (if any), + then we can load them both with one LXVP instruction. */ + if ((regno & 1) == 0 + && VSX_REGNO_P (regno) + && MEM_P (XVECEXP (src, 0, index)) + && MEM_P (XVECEXP (src, 0, index_next))) + { + rtx base = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index) + : XVECEXP (src, 0, index_next); + rtx next = WORDS_BIG_ENDIAN ? XVECEXP (src, 0, index_next) + : XVECEXP (src, 0, index); + + if (consecutive_mem_locations (base, next)) + { + dst_i = gen_rtx_REG (OOmode, regno); + emit_move_insn (dst_i, adjust_address (base, OOmode, 0)); + /* Skip the next register, since we just loaded it. */ + i++; + continue; + } + } + + dst_i = gen_rtx_REG (reg_mode, reg + i); + emit_insn (gen_rtx_SET (dst_i, XVECEXP (src, 0, index))); } /* We are writing an accumulator register, so we have to @@ -18427,23 +18456,37 @@ get_memref_parts (rtx mem, rtx *base, HOST_WIDE_INT *offset, return true; } -/* The function returns true if the target storage location of - mem1 is adjacent to the target storage location of mem2 */ -/* Return 1 if memory locations are adjacent. */ +/* If the target storage locations of arguments MEM1 and MEM2 are + adjacent, then return the argument that has the lower address. + Otherwise, return NULL_RTX. */ -static bool +static rtx adjacent_mem_locations (rtx mem1, rtx mem2) { rtx reg1, reg2; HOST_WIDE_INT off1, size1, off2, size2; if (get_memref_parts (mem1, ®1, &off1, &size1) - && get_memref_parts (mem2, ®2, &off2, &size2)) - return ((REGNO (reg1) == REGNO (reg2)) - && ((off1 + size1 == off2) - || (off2 + size2 == off1))); + && get_memref_parts (mem2, ®2, &off2, &size2) + && REGNO (reg1) == REGNO (reg2)) + { + if (off1 + size1 == off2) + return mem1; + else if (off2 + size2 == off1) + return mem2; + } - return false; + return NULL_RTX; +} + +/* The function returns true if the target storage location of + MEM1 is adjacent to the target storage location of MEM2 and + MEM1 has a lower address then MEM2. */ + +static bool +consecutive_mem_locations (rtx mem1, rtx mem2) +{ + return adjacent_mem_locations (mem1, mem2) == mem1; } /* This function returns true if it can be determined that the two MEM @@ -19009,7 +19052,7 @@ power6_sched_reorder2 (rtx_insn **ready, int lastpos) first_store_pos = pos; if (is_store_insn (last_scheduled_insn, &str_mem2) - && adjacent_mem_locations (str_mem, str_mem2)) + && adjacent_mem_locations (str_mem, str_mem2) != NULL_RTX) { /* Found an adjacent store. Move it to the head of the ready list, and adjust it's priority so that it is diff --git a/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c b/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c new file mode 100644 index 00000000000..397d0f1db35 --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/mma-builtin-9.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target power10_ok } */ +/* { dg-options "-mdejagnu-cpu=power10 -O2" } */ + +typedef unsigned char vec_t __attribute__((vector_size(16))); + +void +foo (__vector_pair *dst, vec_t *src) +{ + __vector_pair pair; + /* Adjacent loads should be combined into one lxvp instruction. */ + __builtin_vsx_build_pair (&pair, src[0], src[1]); + *dst = pair; +} + +void +bar (__vector_quad *dst, vec_t *src) +{ + __vector_quad quad; + /* Adjacent loads should be combined into two lxvp instructions. */ + __builtin_mma_build_acc (&quad, src[0], src[1], src[2], src[3]); + *dst = quad; +} + +/* { dg-final { scan-assembler-not {\mlxv\M} } } */ +/* { dg-final { scan-assembler-not {\mstxv\M} } } */ +/* { dg-final { scan-assembler-times {\mlxvp\M} 3 } } */ +/* { dg-final { scan-assembler-times {\mstxvp\M} 3 } } */