Message ID | AANLkTikj68pxgtZ8dgQ2YRFWh9dsL7-N36A6SL1uQ2Nk@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Tue, Nov 30, 2010 at 3:21 PM, Carrot Wei <carrot@google.com> wrote: > Hi > > An instruction of the following style is 32 bit in thumb2. > and r2, r3, r2 > > If we change the source operands order, it will be 16 bit. > ands r2, r2, r3 > > This patch contains a new peephole2 to detect the situation that the all 3 > operands of AND are low registers, and the target is the same as the second > source, then replace it with another AND with its source operands exchanged. Looking at what thumb1 does: if (GET_CODE (operands[2]) != CONST_INT) { rtx tmp = force_reg (SImode, operands[2]); if (rtx_equal_p (operands[0], operands[1])) operands[2] = tmp; else { operands[2] = operands[1]; operands[1] = tmp; } } I almost think thumb2 should do the same (maybe it should be done always in the arm back-end). Thanks, Andrew Pinski
On Tue, Nov 30, 2010 at 3:28 PM, Andrew Pinski <pinskia@gmail.com> wrote: > On Tue, Nov 30, 2010 at 3:21 PM, Carrot Wei <carrot@google.com> wrote: >> Hi >> >> An instruction of the following style is 32 bit in thumb2. >> and r2, r3, r2 >> >> If we change the source operands order, it will be 16 bit. >> ands r2, r2, r3 >> >> This patch contains a new peephole2 to detect the situation that the all 3 >> operands of AND are low registers, and the target is the same as the second >> source, then replace it with another AND with its source operands exchanged. > > > Looking at what thumb1 does: > if (GET_CODE (operands[2]) != CONST_INT) > { > rtx tmp = force_reg (SImode, operands[2]); > if (rtx_equal_p (operands[0], operands[1])) > operands[2] = tmp; > else > { > operands[2] = operands[1]; > operands[1] = tmp; > } > } > I almost think thumb2 should do the same (maybe it should be done > always in the arm back-end). > That's too early, all are in pseudo registers. I think it may miss some cases. thanks Guozhi
On Tue, Nov 30, 2010 at 4:02 PM, Carrot Wei <carrot@google.com> wrote:
> That's too early, all are in pseudo registers. I think it may miss some cases.
Oh I see it now, I think. Though it might help in general and make
thumb less generic.
-- Pinski
On Tue, 2010-11-30 at 15:21 -0800, Carrot Wei wrote: > Hi > > An instruction of the following style is 32 bit in thumb2. > and r2, r3, r2 > > If we change the source operands order, it will be 16 bit. > ands r2, r2, r3 > > This patch contains a new peephole2 to detect the situation that the all 3 > operands of AND are low registers, and the target is the same as the second > source, then replace it with another AND with its source operands exchanged. > > This patch passed the regression test on arm qemu. > So gas will already generate a 16-bit instruction from ands r2, r3, r2 So it should be possible to just extend the existing peephole/split to handle this case (and for all other commutative operations). R. > thanks > Guozhi > > > ChangeLog: > 2010-11-30 Wei Guozhi <carrot@google.com> > > PR target/46631 > * config/arm/thumb2.md (new peephole2): New peephole2. > > > 2010-11-30 Wei Guozhi <carrot@google.com> > > PR target/46631 > * gcc.target/arm/pr46631: New testcase. > > > Index: thumb2.md > =================================================================== > --- thumb2.md (revision 167257) > +++ thumb2.md (working copy) > @@ -1118,3 +1118,17 @@ > " > operands[2] = GEN_INT (32 - INTVAL (operands[2])); > ") > + > +(define_peephole2 > + [(set (match_operand:SI 0 "low_register_operand" "") > + (and:SI (match_operand:SI 1 "low_register_operand" "") > + (match_dup 0)))] > + "TARGET_THUMB2 && peep2_regno_dead_p(0, CC_REGNUM)" > + [(parallel > + [(set (match_dup 0) > + (and:SI (match_dup 0) > + (match_dup 1))) > + (clobber (reg:CC CC_REGNUM))])] > + "" > +) > + > > > Index: pr46631.c > =================================================================== > --- pr46631.c (revision 0) > +++ pr46631.c (revision 0) > @@ -0,0 +1,16 @@ > +/* { dg-options "-mthumb -Os" } */ > +/* { dg-require-effective-target arm_thumb2_ok } */ > +/* { dg-final { scan-assembler "ands" } } */ > + > +struct S { > + int bi_buf; > + int bi_valid; > +}; > + > +int tz (struct S* p, int bits, int value) > +{ > + if (p == 0) return 1; > + p->bi_valid = bits; > + p->bi_buf = value & ((1 << bits) - 1); > + return 0; > +}
Index: thumb2.md =================================================================== --- thumb2.md (revision 167257) +++ thumb2.md (working copy) @@ -1118,3 +1118,17 @@ " operands[2] = GEN_INT (32 - INTVAL (operands[2])); ") + +(define_peephole2 + [(set (match_operand:SI 0 "low_register_operand" "") + (and:SI (match_operand:SI 1 "low_register_operand" "") + (match_dup 0)))] + "TARGET_THUMB2 && peep2_regno_dead_p(0, CC_REGNUM)" + [(parallel + [(set (match_dup 0) + (and:SI (match_dup 0) + (match_dup 1))) + (clobber (reg:CC CC_REGNUM))])] + "" +) + Index: pr46631.c =================================================================== --- pr46631.c (revision 0) +++ pr46631.c (revision 0) @@ -0,0 +1,16 @@ +/* { dg-options "-mthumb -Os" } */ +/* { dg-require-effective-target arm_thumb2_ok } */ +/* { dg-final { scan-assembler "ands" } } */ + +struct S { + int bi_buf; + int bi_valid; +}; + +int tz (struct S* p, int bits, int value) +{ + if (p == 0) return 1; + p->bi_valid = bits; + p->bi_buf = value & ((1 << bits) - 1); + return 0; +}