| Message ID | 5034EBC6.5080602@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Sandra Loosemore <sandra@codesourcery.com> writes: > On 08/19/2012 11:22 AM, Richard Sandiford wrote: >> >> Not sure whether a peephole is the right choice here. In practice, >> I'd imagine these opportunities would only come from a DImode move of >> $0 into a doubleword register, so we could simply emit the pattern in >> mips_split_doubleword_move. >> >> That would also allow us to use it for plain HI and LO. It wasn't >> obvious from the patch why it was restricted to the DSP extension >> registers. >> >> Please also add a scan-assembler test. > > How is this version of the fix? Just to say that I've not forgotten about this. I'd still like to remove the !TARGET_64BIT and ISA_HAS_DSP_MULT tests, because the idea isn't specific to either. Also, reviewing the patch made me realise that it might be better to keep the move intact and simply use "mult" in the output code. That's my fault for suggesting the wrong thing, though, so I was hoping to find time this weekend to try it myself. The testsuite stuff ended up taking up all the available time instead. Richard
On 08/27/2012 10:36 AM, Richard Sandiford wrote: > Sandra Loosemore<sandra@codesourcery.com> writes: >> On 08/19/2012 11:22 AM, Richard Sandiford wrote: >>> >>> Not sure whether a peephole is the right choice here. In practice, >>> I'd imagine these opportunities would only come from a DImode move of >>> $0 into a doubleword register, so we could simply emit the pattern in >>> mips_split_doubleword_move. >>> >>> That would also allow us to use it for plain HI and LO. It wasn't >>> obvious from the patch why it was restricted to the DSP extension >>> registers. >>> >>> Please also add a scan-assembler test. >> >> How is this version of the fix? > > Just to say that I've not forgotten about this. I'd still like to > remove the !TARGET_64BIT and ISA_HAS_DSP_MULT tests, because the > idea isn't specific to either. > > Also, reviewing the patch made me realise that it might be better > to keep the move intact and simply use "mult" in the output code. > That's my fault for suggesting the wrong thing, though, so I was > hoping to find time this weekend to try it myself. The testsuite > stuff ended up taking up all the available time instead. Richard, Have you had time to think about this some more? I am not sure I can guess how you'd like me to fix this patch now without some more specific review and/or suggestions about where the optimization should happen and what cases it should be extended to detect in addition to the dsp accumulator multiplies. -Sandra
Index: gcc/config/mips/mips.c =================================================================== --- gcc/config/mips/mips.c (revision 190463) +++ gcc/config/mips/mips.c (working copy) @@ -4158,6 +4158,14 @@ mips_split_doubleword_move (rtx dest, rt else emit_insn (gen_mfhisi_di (mips_subword (dest, true), src)); } + else if (!TARGET_64BIT && !TARGET_MIPS16 && ISA_HAS_DSP_MULT + && src == const0_rtx + && REG_P (dest) && ACC_REG_P (REGNO (dest))) + /* Zero-initialize accumulator using "mult $dest,$0,$0" instead + of a mthi/mtlo pair. */ + emit_insn (gen_mulsidi3_32bit (dest, + gen_rtx_REG (SImode, GP_REG_FIRST), + gen_rtx_REG (SImode, GP_REG_FIRST))); else { /* The operation can be split into two normal moves. Decide in Index: gcc/testsuite/gcc.target/mips/mips32-dsp-accinit.c =================================================================== --- gcc/testsuite/gcc.target/mips/mips32-dsp-accinit.c (revision 0) +++ gcc/testsuite/gcc.target/mips/mips32-dsp-accinit.c (revision 0) @@ -0,0 +1,16 @@ +/* { dg-options "-O2 -march=74kc -mgp32" } */ + +/* Check that the zero-initialization of the accumulator feeding into + the madd is done by means of a mult instruction instead of mthi/mtlo. */ + +NOMIPS16 long long f (int n, int *v, int m) +{ + long long result = 0; + int i; + + for (i = 0; i < n; i++) + result = __builtin_mips_madd (result, v[i], m); + return result; +} + +/* { dg-final { scan-assembler "mult\t\\\$ac.,\\\$0,\\\$0" } } */