Message ID | CA+=Sn1nNxA+D1GQAbG84YWKmZc8eu=MbD91aiJHTmy2gT4S_Vw@mail.gmail.com |
---|---|
State | New |
Headers | show |
Andrew Pinski <andrew.pinski@caviumnetworks.com> writes: > 2012-07-26 Andrew Pinski <apinski@cavium.com> > > Bug #3261 > * config/mips/mips.md (*mov<GPR:mode>_on_<MOVECC:mode>): > Remove mode check from comparisons. > (*mov<SCALARF:mode>_on_<MOVECC:mode>): Likewise. > (*mov<GPR:mode>_on_<GPR2:mode>_ne): New pattern to match > when (ne A 0) can be just A. > > * testsuite/gcc.target/mips/movcc-4.c: New testcase. OK, thanks (but remember to remove the internal bug reference :-)). I think this is early enough during stage 3 for the usual target flexibility to apply. Richard
On Thu, Nov 15, 2012 at 12:58 PM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > Andrew Pinski <andrew.pinski@caviumnetworks.com> writes: >> 2012-07-26 Andrew Pinski <apinski@cavium.com> >> >> Bug #3261 >> * config/mips/mips.md (*mov<GPR:mode>_on_<MOVECC:mode>): >> Remove mode check from comparisons. >> (*mov<SCALARF:mode>_on_<MOVECC:mode>): Likewise. >> (*mov<GPR:mode>_on_<GPR2:mode>_ne): New pattern to match >> when (ne A 0) can be just A. >> >> * testsuite/gcc.target/mips/movcc-4.c: New testcase. > > OK, thanks (but remember to remove the internal bug reference :-)). > I think this is early enough during stage 3 for the usual target > flexibility to apply. I was posting it for Steve's benefit really. I was in the process of updating the patch to the trunk and trying it out there before doing a formal submission :). As I found out the testcase needs to be changed to work with the new mips target test infrastructure. I will post a revised patch with the removal of the internal bug number once I finish fixing the testcase itself. Thanks, Andrew Pinski
On Thu, Nov 15, 2012 at 1:24 PM, Andrew Pinski <andrew.pinski@caviumnetworks.com> wrote: > On Thu, Nov 15, 2012 at 12:58 PM, Richard Sandiford > <rdsandiford@googlemail.com> wrote: >> Andrew Pinski <andrew.pinski@caviumnetworks.com> writes: >>> 2012-07-26 Andrew Pinski <apinski@cavium.com> >>> >>> Bug #3261 >>> * config/mips/mips.md (*mov<GPR:mode>_on_<MOVECC:mode>): >>> Remove mode check from comparisons. >>> (*mov<SCALARF:mode>_on_<MOVECC:mode>): Likewise. >>> (*mov<GPR:mode>_on_<GPR2:mode>_ne): New pattern to match >>> when (ne A 0) can be just A. >>> >>> * testsuite/gcc.target/mips/movcc-4.c: New testcase. >> >> OK, thanks (but remember to remove the internal bug reference :-)). >> I think this is early enough during stage 3 for the usual target >> flexibility to apply. > > I was posting it for Steve's benefit really. I was in the process of > updating the patch to the trunk and trying it out there before doing a > formal submission :). As I found out the testcase needs to be changed > to work with the new mips target test infrastructure. I will post a > revised patch with the removal of the internal bug number once I > finish fixing the testcase itself. After fixing up the testcase I find xori still in the resulting code: gcc$ ./cc1 t.c -O1 -o - -DNOMIPS16= -quiet -mabi=n32 -march=mips64 |grep xor xori $2,$4,0x1 But that is because combine produces: Trying 34 -> 35: Failed to match this instruction: (set (reg:SI 194 [ D.1393 ]) (if_then_else:SI (xor:SI (reg:SI 200 [ d ]) (const_int 1 [0x1])) (reg:SI 6 $6 [ c ]) (reg:SI 5 $5 [ b ]))) But does not switch around the if_then_else as reg 200 has a non zero bits of just 1. I will look into fix the rest of the problem later today. So the above patch is a way forward but not the full fix. Thanks, Andrew Pinski
On Thu, 2012-11-15 at 13:39 -0800, Andrew Pinski wrote: > > I was posting it for Steve's benefit really. I was in the process of > > updating the patch to the trunk and trying it out there before doing a > > formal submission :). As I found out the testcase needs to be changed > > to work with the new mips target test infrastructure. I will post a > > revised patch with the removal of the internal bug number once I > > finish fixing the testcase itself. > > After fixing up the testcase I find xori still in the resulting code: > gcc$ ./cc1 t.c -O1 -o - -DNOMIPS16= -quiet -mabi=n32 -march=mips64 |grep xor > xori $2,$4,0x1 > > But that is because combine produces: > Trying 34 -> 35: > Failed to match this instruction: > (set (reg:SI 194 [ D.1393 ]) > (if_then_else:SI (xor:SI (reg:SI 200 [ d ]) > (const_int 1 [0x1])) > (reg:SI 6 $6 [ c ]) > (reg:SI 5 $5 [ b ]))) > > But does not switch around the if_then_else as reg 200 has a non zero > bits of just 1. I will look into fix the rest of the problem later > today. So the above patch is a way forward but not the full fix. > > Thanks, > Andrew Pinski Andrew, are you still planning on submitting this patch? I have been running with your new "*mov<GPR:mode>_on_<GPR2:mode>_ne" instruction and that part at least works fine. I would like to get at least that much checked in for GCC 4.8. Steve Ellcey sellcey@mips.com
On Wed, Nov 14, 2012 at 02:22:33PM -0800, Andrew Pinski wrote: > commit 8ca1e58de404bbe82b93bc240ef28c68c681243d > Author: Andrew Pinski <apinski@cavium.com> > Date: Thu Jul 26 18:09:34 2012 -0700 > > 2012-07-26 Andrew Pinski <apinski@cavium.com> > > Bug #3261 > * config/mips/mips.md (*mov<GPR:mode>_on_<MOVECC:mode>): > Remove mode check from comparisons. > (*mov<SCALARF:mode>_on_<MOVECC:mode>): Likewise. > (*mov<GPR:mode>_on_<GPR2:mode>_ne): New pattern to match > when (ne A 0) can be just A. Why aren't you also adding a *mov<SCALARF:mode>_on_<GPR2:mode>_ne insn? Jakub
On Thu, Mar 7, 2013 at 7:12 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, Nov 14, 2012 at 02:22:33PM -0800, Andrew Pinski wrote: >> commit 8ca1e58de404bbe82b93bc240ef28c68c681243d >> Author: Andrew Pinski <apinski@cavium.com> >> Date: Thu Jul 26 18:09:34 2012 -0700 >> >> 2012-07-26 Andrew Pinski <apinski@cavium.com> >> >> Bug #3261 >> * config/mips/mips.md (*mov<GPR:mode>_on_<MOVECC:mode>): >> Remove mode check from comparisons. >> (*mov<SCALARF:mode>_on_<MOVECC:mode>): Likewise. >> (*mov<GPR:mode>_on_<GPR2:mode>_ne): New pattern to match >> when (ne A 0) can be just A. > > Why aren't you also adding a *mov<SCALARF:mode>_on_<GPR2:mode>_ne > insn? Most likely because I only tested performance of this patch on soft-float and I did not notice a reason for it yet. Thanks, Andrew Pinski
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index 0dff58e..a1e9568 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -6765,7 +6765,7 @@ (define_insn "*mov<GPR:mode>_on_<MOVECC:mode>" [(set (match_operand:GPR 0 "register_operand" "=d,d") (if_then_else:GPR - (match_operator:MOVECC 4 "equality_operator" + (match_operator 4 "equality_operator" [(match_operand:MOVECC 1 "register_operand" "<MOVECC:reg>,<MOVECC:reg>") (const_int 0)]) (match_operand:GPR 2 "reg_or_0_operand" "dJ,0") @@ -6777,10 +6777,23 @@ [(set_attr "type" "condmove") (set_attr "mode" "<GPR:MODE>")]) +(define_insn "*mov<GPR:mode>_on_<GPR2:mode>_ne" + [(set (match_operand:GPR 0 "register_operand" "=d,d") + (if_then_else:GPR + (match_operand:GPR2 1 "register_operand" "<GPR2:reg>,<GPR2:reg>") + (match_operand:GPR 2 "reg_or_0_operand" "dJ,0") + (match_operand:GPR 3 "reg_or_0_operand" "0,dJ")))] + "ISA_HAS_CONDMOVE" + "@ + movn\t%0,%z2,%1 + movz\t%0,%z3,%1" + [(set_attr "type" "condmove") + (set_attr "mode" "<GPR:MODE>")]) + (define_insn "*mov<SCALARF:mode>_on_<MOVECC:mode>" [(set (match_operand:SCALARF 0 "register_operand" "=f,f") (if_then_else:SCALARF - (match_operator:MOVECC 4 "equality_operator" + (match_operator 4 "equality_operator" [(match_operand:MOVECC 1 "register_operand" "<MOVECC:reg>,<MOVECC:reg>") (const_int 0)]) (match_operand:SCALARF 2 "register_operand" "f,0") diff --git a/gcc/testsuite/gcc.target/mips/movcc-4.c b/gcc/testsuite/gcc.target/mips/movcc-4.c new file mode 100644 index 0000000..d364a52 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/movcc-4.c @@ -0,0 +1,13 @@ +/* { dg-options "-O2 isa>=4" } */ +/* { dg-final { scan-assembler-times "movz\t|movn\t" 1 } } */ +/* { dg-final { scan-assembler-not "bbit0\t|bbit1\t" } } */ +/* { dg-final { scan-assembler-not "xori\t|xor\t" } } */ + +NOMIPS16 int f(int a, int b, int c) +{ + int d = a&0x1; + if (d==1) + return b; + return c; +} +