Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 19 August 2011 11:06, Ramana Radhakrishnan
ramana.radhakrish...@linaro.org wrote:
Regression test against cortex-M0/M3/M4 profile with -mthumb option
doesn't show any new failures.
Please test on ARM state as well and make sure there are no
regressions before committing.
Jiangning told me privately that the test-results for v7-a were fine
for cross-testing for arm-eabi with C and C++.
And this is what I committed
cheers
Ramana
2011-08-26 Jiangning Liu jiangning@arm.com
* config/arm/arm.md (*ior_scc_scc): Enable for Thumb2 as well.
(*ior_scc_scc_cmp): Likewise
(*and_scc_scc): Likewise.
(*and_scc_scc_cmp): Likewise.
(*and_scc_scc_nodom): Likewise.
(*cmp_ite0, *cmp_ite1, *cmp_and, *cmp_ior): Handle Thumb2.
2011-08-26 Jiangning Liu jiangning@arm.com
* gcc.target/arm/thumb2-cond-cmp-1.c: New.
* gcc.target/arm/thumb2-cond-cmp-2.c: Likewise.
* gcc.target/arm/thumb2-cond-cmp-3.c: Likewise.
* gcc.target/arm/thumb2-cond-cmp-4.c: Likewise.
Ok if no regressions.
Ramana
Thanks,
-Jiangning
Index: gcc/config/arm/arm.md
===
--- gcc/config/arm/arm.md (revision 178097)
+++ gcc/config/arm/arm.md (working copy)
@@ -49,6 +49,15 @@
(DOM_CC_X_OR_Y 2)
]
)
+;; conditional compare combination
+(define_constants
+ [(CMP_CMP 0)
+ (CMN_CMP 1)
+ (CMP_CMN 2)
+ (CMN_CMN 3)
+ (NUM_OF_COND_CMP 4)
+ ]
+)
;; UNSPEC Usage:
;; Note: sin and cos are no-longer used.
@@ -8980,40 +8989,85 @@
(set_attr length 8,12)]
)
-;; ??? Is it worth using these conditional patterns in Thumb-2 mode?
(define_insn *cmp_ite0
[(set (match_operand 6 dominant_cc_register )
(compare
(if_then_else:SI
(match_operator 4 arm_comparison_operator
- [(match_operand:SI 0 s_register_operand r,r,r,r)
- (match_operand:SI 1 arm_add_operand rI,L,rI,L)])
+ [(match_operand:SI 0 s_register_operand
+ l,l,l,r,r,r,r,r,r)
+ (match_operand:SI 1 arm_add_operand
+ lPy,lPy,lPy,rI,L,rI,L,rI,L)])
(match_operator:SI 5 arm_comparison_operator
- [(match_operand:SI 2 s_register_operand r,r,r,r)
- (match_operand:SI 3 arm_add_operand rI,rI,L,L)])
+ [(match_operand:SI 2 s_register_operand
+ l,r,r,l,l,r,r,r,r)
+ (match_operand:SI 3 arm_add_operand
+ lPy,rI,L,lPy,lPy,rI,rI,L,L)])
(const_int 0))
(const_int 0)))]
- TARGET_ARM
+ TARGET_32BIT
*
{
-static const char * const opcodes[4][2] =
+static const char * const cmp1[NUM_OF_COND_CMP][2] =
{
- {\cmp\\t%2, %3\;cmp%d5\\t%0, %1\,
- \cmp\\t%0, %1\;cmp%d4\\t%2, %3\},
- {\cmp\\t%2, %3\;cmn%d5\\t%0, #%n1\,
- \cmn\\t%0, #%n1\;cmp%d4\\t%2, %3\},
- {\cmn\\t%2, #%n3\;cmp%d5\\t%0, %1\,
- \cmp\\t%0, %1\;cmn%d4\\t%2, #%n3\},
- {\cmn\\t%2, #%n3\;cmn%d5\\t%0, #%n1\,
- \cmn\\t%0, #%n1\;cmn%d4\\t%2, #%n3\}
+ {\cmp%d5\\t%0, %1\,
+ \cmp%d4\\t%2, %3\},
+ {\cmn%d5\\t%0, #%n1\,
+ \cmp%d4\\t%2, %3\},
+ {\cmp%d5\\t%0, %1\,
+ \cmn%d4\\t%2, #%n3\},
+ {\cmn%d5\\t%0, #%n1\,
+ \cmn%d4\\t%2, #%n3\}
};
+static const char * const cmp2[NUM_OF_COND_CMP][2] =
+{
+ {\cmp\\t%2, %3\,
+ \cmp\\t%0, %1\},
+ {\cmp\\t%2, %3\,
+ \cmn\\t%0, #%n1\},
+ {\cmn\\t%2, #%n3\,
+ \cmp\\t%0, %1\},
+ {\cmn\\t%2, #%n3\,
+ \cmn\\t%0, #%n1\}
+};
+static const char * const ite[2] =
+{
+ \it\\t%d5\,
+ \it\\t%d4\
+};
+static const int cmp_idx[9] = {CMP_CMP, CMP_CMP, CMP_CMN,
+ CMP_CMP, CMN_CMP, CMP_CMP,
+ CMN_CMP, CMP_CMN, CMN_CMN};
int swap =
comparison_dominates_p (GET_CODE (operands[5]), GET_CODE (operands[4]));
-return opcodes[which_alternative][swap];
+output_asm_insn (cmp2[cmp_idx[which_alternative]][swap], operands);
+if (TARGET_THUMB2) {
+ output_asm_insn (ite[swap], operands);
+}
+output_asm_insn (cmp1[cmp_idx[which_alternative]][swap], operands);
+return \\;
}
[(set_attr conds set)
- (set_attr length 8)]
+ (set_attr arch t2,t2,t2,t2,t2,any,any,any,any)
+ (set_attr_alternative length
+ [(const_int 6)
+ (const_int 8)
+ (const_int 8)
+ (const_int 8)
+ (const_int 8)
+ (if_then_else (eq_attr is_thumb no)
+ (const_int 8)
+ (const_int 10))
+ (if_then_else (eq_attr is_thumb no)
+ (const_int 8)
+ (const_int 10))
+ (if_then_else (eq_attr is_thumb no)
+ (const_int 8)
+ (const_int 10))
+ (if_then_else (eq_attr is_thumb no)
+ (const_int 8)
+ (const_int 10))])]
)
(define_insn *cmp_ite1
@@ -9021,35 +9075,81 @@
(compare
(if_then_else:SI
Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
Regression test against cortex-M0/M3/M4 profile with -mthumb option doesn't show any new failures. Please test on ARM state as well and make sure there are no regressions before committing. Ok if no regressions. Ramana Thanks, -Jiangning
RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
Attached is the new patch file. Review please! ChangeLog: 2011-08-18 Jiangning Liu jiangning@arm.com * config/arm/arm.md (*ior_scc_scc): Enable for Thumb2 as well. (*ior_scc_scc_cmp): Likewise (*and_scc_scc): Likewise. (*and_scc_scc_cmp): Likewise. (*and_scc_scc_nodom): Likewise. (*cmp_ite0, *cmp_ite1, *cmp_and, *cmp_ior): Handle Thumb2. Testsuite Changelog would be: 2011-08-18 Jiangning Liu jiangning@arm.com * gcc.target/arm/thumb2-cond-cmp-1.c: New. Make sure conditional compare can be generated. * gcc.target/arm/thumb2-cond-cmp-2.c: Likewise. * gcc.target/arm/thumb2-cond-cmp-3.c: Likewise. * gcc.target/arm/thumb2-cond-cmp-4.c: Likewise. Regression test against cortex-M0/M3/M4 profile with -mthumb option doesn't show any new failures. Thanks, -Jiangning fix_cond_cmp_5.patch Description: Binary data
RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
Ramana,
I only see the following three combinations are meaningful,
* cmp l, lPy // keep cmp, and length is 2, on thumb
* cmp r, rI // keep cmp, and length is 4
* cmp r, L // convert to cmn, and length is 4
According to ARM ARM, for negative immediate, all encodings for cmp/cmn are
4-byte long, i.e.
* CMP: encoding T2 and A1
* CMN: encoding T1 and A1
so we needn't to make difference to cover Pw and Pv.
Length is 8 bytes if you have Pw and L
For this cases, the length should be 10 for Thumb2 instead.
Finally, if we want to describe all possibilities in constraints, we would
have the followings 9 combinations,
* cmp1 has operands
(l, l, l, r, r, r, r, r, r)
(lPy,lPy,lPy,rI,rI,rI,L, L, L)
* cmp2 has operands
(l, r, r, l, r, r, l, r, r)
(lPy,rI,L, lPy,rI,L, lPy,rI,L)
So the length would be like below, (I don't know how to write it in
attribute section yet. )
if (TARGET_THUMB2) {
(set_attr length 6,8,8,8,10,10,8,10,10)]
} else {
(set_attr length 4,6,6,6,8,8,6,8,8)]
}
Does it make sense?
Thanks,
-Jiangning
-Original Message-
From: Ramana Radhakrishnan [mailto:ramana.radhakrish...@linaro.org]
Sent: Wednesday, August 10, 2011 6:40 PM
To: Jiangning Liu
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 10 August 2011 09:20, Jiangning Liu jiangning@arm.com wrote:
PING...
BTW, in patch fix_cond_cmp_2.patch, the file mode of thumb2.md is
carelessly
changed, so please check attached new patch file fix_cond_cmp_3.patch.
Please do not top post.
I've been away for the whole of last week and then been away from my
desk
for the first 2 days this week and am still catching up with email .
I'm missing a Changelog entry for this . Do I assume it is the same ?
I've just noticed that the length attribute isn't correct for Thumb2
state. When I last
looked at this I must have missed the case where the cmp and the cmn
are both 32 bit instructions.
The length can vary between 6 and 10 bytes for the Thumb2 variant from
my reading of the ARM-ARM.
i.e cmp reg, 8bitconstant
itcond
cmn reg, 8bitconstant
Length = 6 bytes
or even with
cmp reg, reg
it cond
cmncond reg, reg
All registers betwen r0 and r7 .
Length is 6 bytes if you have this for both l constraints.
Length is 6 bytes - you should catch this with Pw and Pv respectively .
Length is 8 bytes if you have Pw and L
Length is 8 bytes if you have I and Pv
Length is 10 bytes if you have I and L .
Showing you an example with l and Py at this point of time and leaving
the rest as homework. Yes it will duplicate a few cases but it helps
getting the lengths absolutely right.
(define_insn *cmp_ite0
[(set (match_operand 6 dominant_cc_register )
(compare
(if_then_else:SI
(match_operator 4 arm_comparison_operator
[(match_operand:SI 0 s_register_operand l,r,r,r,r)
(match_operand:SI 1 arm_add_operand lPy,rI,L,rI,L)])
(match_operator:SI 5 arm_comparison_operator
[(match_operand:SI 2 s_register_operand l,r,r,r,r)
(match_operand:SI 3 arm_add_operand lPy,rI,rI,L,L)])
(const_int 0))
(const_int 0)))]
TARGET_32BIT
*
{
static const char * const cmp1[5][2] =
{
{\cmp\\t%2, %3\,
\cmp\\t%0, %1\},
{\cmp\\t%2, %3\,
\cmp\\t%0, %1\},
{\cmp\\t%2, %3\,
\cmn\\t%0, #%n1\},
{\cmn\\t%2, #%n3\,
\cmp\\t%0, %1\},
{\cmn\\t%2, #%n3\,
\cmn\\t%0, #%n1\}
};
static const char * const cmp2[5][2] =
{
{\cmp%d5\\t%0, %1\,
\cmp%d4\\t%2, %3\},
{\cmp%d5\\t%0, %1\,
\cmp%d4\\t%2, %3\},
{\cmn%d5\\t%0, #%n1\,
\cmp%d4\\t%2, %3\},
{\cmp%d5\\t%0, %1\,
\cmn%d4\\t%2, #%n3\},
{\cmn%d5\\t%0, #%n1\,
\cmn%d4\\t%2, #%n3\}
};
static const char * const ite[2] =
{
\it\\t%d5\,
\it\\t%d4\
};
int swap =
comparison_dominates_p (GET_CODE (operands[5]), GET_CODE
(operands[4]));
output_asm_insn (cmp1[which_alternative][swap], operands);
if (TARGET_THUMB2) {
output_asm_insn (ite[swap], operands);
}
output_asm_insn (cmp2[which_alternative][swap], operands);
return \\;
}
[(set_attr conds set)
(set_attr arch t2,any,any,any,any)
(set_attr length 6,8,8,8,8)]
)
As for the extra problem exposed by this specific case, may we treat
it as a
separate fix to decouple it with this one, and I can give follow up
later
on? I think it is a general problem not only for the particular
pattern
it/op/it/op. But I'm not sure how far we can go to optimize this kind
of
problems introduced by IT block.
Currently the way in which the Thumb2 backend generates
conditional instructions and combines them further with other
IT blocks is by running a state machine at the very end before assembly
RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
PING...
BTW, in patch fix_cond_cmp_2.patch, the file mode of thumb2.md is carelessly
changed, so please check attached new patch file fix_cond_cmp_3.patch.
Thanks,
-Jiangning
-Original Message-
From: Jiangning Liu [mailto:jiangning@arm.com]
Sent: Monday, August 08, 2011 2:01 PM
To: 'Ramana Radhakrishnan'
Cc: gcc-patches@gcc.gnu.org
Subject: RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
In attached new patch, arm_arch_thumb2 is changed to TARGET_THUMB2. I
tried that with my patch command line option -mcpu=armv7-a9 doesn't
generate IT instruction any longer, unless option -mthumb is being
added.
All of my tests assume command line option -mthumb, while cortex-M0,
cortex-M3 cortex-M4 are covered by options -mcpu=cortex-m0, -
march=armv7-m, and -march=armv7e-m respectively.
As for the extra problem exposed by this specific case, may we treat it
as a separate fix to decouple it with this one, and I can give follow
up later on? I think it is a general problem not only for the
particular pattern it/op/it/op. But I'm not sure how far we can go to
optimize this kind of problems introduced by IT block. For this
specific case, I see if conversion already generates conditional move
before combination pass. So basically the peephole rules may probably
work for most of the general scenarios. My initial thought is go over
the rules introducing IT block and try to figure out all of the
combination that two of this kinds of rules can be in sequential order.
Maybe we only need to handle the most common case like this one. Since
I do see a bunch of rules have something to do with problem, I'd like
to look into all of them to give a most reasonable solution in a
separate fix.
Does it make sense?
Thanks,
-Jiangning
-Original Message-
From: Ramana Radhakrishnan [mailto:ramana.radhakrish...@linaro.org]
Sent: Friday, August 05, 2011 9:20 AM
To: Jiangning Liu
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2
state
On 3 August 2011 08:48, Jiangning Liu jiangning@arm.com wrote:
This patch is to generate more conditional compare instructions in
Thumb2
state. Given an example like below,
int f(int i, int j)
{
if ( (i == '+') || (j == '-') ) {
return i;
} else {
return j;
}
}
Without the patch, compiler generates the following codes,
sub r2, r0, #43
rsbs r3, r2, #0
adc r3, r3, r2
cmp r1, #45
it eq
orreq r3, r3, #1
cmp r3, #0
it eq
moveq r0, r1
bx lr
With the patch, compiler can generate conditional jump like below,
cmp r0, #43
it ne
cmpne r1, #45
it ne
movne r0, r1
bx lr
Nice improvement but there could be a single it block to handle both
and thus you could make this even better with
cmp r0, #43
itt ne
cmpne r1 ,#45
movne r0, r1
The way to do this would be to try and split this post-reload
unfortunately into the cmp instruction and the conditional compare
with the appropriate instruction length - Then the backend has a
chance of merging some of this into a single instruction.
Unfortunately that won't be very straight-forward but that's a
direction we probably ought to proceed with in this case.
In a number of places:
+ if (arm_arch_thumb2)
Ah instead of this please use if (TARGET_THUMB2) - arm_arch_thumb2 is
true based on the architecture levels and not necessarily if the user
wants to generate Thumb code. I don't want an unnecessary IT
instruction being emitted in the ASM block in ARM state for v7-a and
above.
Tested against arm-none-eabi target and no regression found.
Presumably for ARM and Thumb2 state ?
cheers
Ramana
fix_cond_cmp_3.patch
Description: Binary data
RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
In attached new patch, arm_arch_thumb2 is changed to TARGET_THUMB2. I tried
that with my patch command line option -mcpu=armv7-a9 doesn't generate IT
instruction any longer, unless option -mthumb is being added.
All of my tests assume command line option -mthumb, while cortex-M0,
cortex-M3 cortex-M4 are covered by options -mcpu=cortex-m0, -march=armv7-m,
and -march=armv7e-m respectively.
As for the extra problem exposed by this specific case, may we treat it as a
separate fix to decouple it with this one, and I can give follow up later
on? I think it is a general problem not only for the particular pattern
it/op/it/op. But I'm not sure how far we can go to optimize this kind of
problems introduced by IT block. For this specific case, I see if
conversion already generates conditional move before combination pass. So
basically the peephole rules may probably work for most of the general
scenarios. My initial thought is go over the rules introducing IT block and
try to figure out all of the combination that two of this kinds of rules can
be in sequential order. Maybe we only need to handle the most common case
like this one. Since I do see a bunch of rules have something to do with
problem, I'd like to look into all of them to give a most reasonable
solution in a separate fix.
Does it make sense?
Thanks,
-Jiangning
-Original Message-
From: Ramana Radhakrishnan [mailto:ramana.radhakrish...@linaro.org]
Sent: Friday, August 05, 2011 9:20 AM
To: Jiangning Liu
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 3 August 2011 08:48, Jiangning Liu jiangning@arm.com wrote:
This patch is to generate more conditional compare instructions in
Thumb2
state. Given an example like below,
int f(int i, int j)
{
if ( (i == '+') || (j == '-') ) {
return i;
} else {
return j;
}
}
Without the patch, compiler generates the following codes,
sub r2, r0, #43
rsbs r3, r2, #0
adc r3, r3, r2
cmp r1, #45
it eq
orreq r3, r3, #1
cmp r3, #0
it eq
moveq r0, r1
bx lr
With the patch, compiler can generate conditional jump like below,
cmp r0, #43
it ne
cmpne r1, #45
it ne
movne r0, r1
bx lr
Nice improvement but there could be a single it block to handle both
and thus you
could make this even better with
cmp r0, #43
itt ne
cmpne r1 ,#45
movne r0, r1
The way to do this would be to try and split this post-reload
unfortunately into the cmp instruction and the conditional compare
with the appropriate instruction length - Then the backend has a
chance of merging some of this into a single instruction.
Unfortunately that won't be very straight-forward but that's a
direction we probably ought to proceed with in this case.
In a number of places:
+ if (arm_arch_thumb2)
Ah instead of this please use if (TARGET_THUMB2) - arm_arch_thumb2 is
true based on the architecture levels and not necessarily if the user
wants to generate Thumb code. I don't want an unnecessary IT
instruction being emitted in the ASM block in ARM state for v7-a and
above.
Tested against arm-none-eabi target and no regression found.
Presumably for ARM and Thumb2 state ?
cheers
Ramana
fix_cond_cmp_2.patch
Description: Binary data
Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 08/05/2011 03:19 AM, Ramana Radhakrishnan wrote: cmp r0, #43 it ne cmpne r1, #45 it ne movne r0, r1 bx lr [...] there could be a single it block to handle both and thus you could make this even better with cmp r0, #43 itt ne cmpne r1 ,#45 movne r0, r1 Conversion from it/op/it/op to itt/op/op (and likewise for it/op/ie/op) seems to be a good fit for a machine-dependent reorg pass. Paolo
Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
Conversion from it/op/it/op to itt/op/op (and likewise for it/op/ie/op) seems to be a good fit for a machine-dependent reorg pass. It does happen in cases where you have proper cond-exec patterns. It's final pre-scan that manages this though there is probably a win in doing this before constant pool placements in md_reorg. If we could represent this in rtl as cond-exec I think that will just pick this up. Ramana Paolo
Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 3 August 2011 08:48, Jiangning Liu jiangning@arm.com wrote:
This patch is to generate more conditional compare instructions in Thumb2
state. Given an example like below,
int f(int i, int j)
{
if ( (i == '+') || (j == '-') ) {
return i;
} else {
return j;
}
}
Without the patch, compiler generates the following codes,
sub r2, r0, #43
rsbs r3, r2, #0
adc r3, r3, r2
cmp r1, #45
it eq
orreq r3, r3, #1
cmp r3, #0
it eq
moveq r0, r1
bx lr
With the patch, compiler can generate conditional jump like below,
cmp r0, #43
it ne
cmpne r1, #45
it ne
movne r0, r1
bx lr
Nice improvement but there could be a single it block to handle both
and thus you
could make this even better with
cmp r0, #43
itt ne
cmpne r1 ,#45
movne r0, r1
The way to do this would be to try and split this post-reload
unfortunately into the cmp instruction and the conditional compare
with the appropriate instruction length - Then the backend has a
chance of merging some of this into a single instruction.
Unfortunately that won't be very straight-forward but that's a
direction we probably ought to proceed with in this case.
In a number of places:
+ if (arm_arch_thumb2)
Ah instead of this please use if (TARGET_THUMB2) - arm_arch_thumb2 is
true based on the architecture levels and not necessarily if the user
wants to generate Thumb code. I don't want an unnecessary IT
instruction being emitted in the ASM block in ARM state for v7-a and
above.
Tested against arm-none-eabi target and no regression found.
Presumably for ARM and Thumb2 state ?
cheers
Ramana
[PATCH, ARM] Generate conditional compares in Thumb2 state
This patch is to generate more conditional compare instructions in Thumb2
state. Given an example like below,
int f(int i, int j)
{
if ( (i == '+') || (j == '-') ) {
return i;
} else {
return j;
}
}
Without the patch, compiler generates the following codes,
sub r2, r0, #43
rsbsr3, r2, #0
adc r3, r3, r2
cmp r1, #45
it eq
orreq r3, r3, #1
cmp r3, #0
it eq
moveq r0, r1
bx lr
With the patch, compiler can generate conditional jump like below,
cmp r0, #43
it ne
cmpne r1, #45
it ne
movne r0, r1
bx lr
The patch is essentially to insert *it* instruction for the following rules
in arm.md,
* cmp_ite0
* cmp_ite1
* cmp_and
* cmp_ior
Tested against arm-none-eabi target and no regression found.
Source code Changelog would be:
2011-07-29 Jiangning Liu jiangning@arm.com
* config/arm/arm.md (*ior_scc_scc): Enable for Thumb2 as well.
(*ior_scc_scc_cmp): Likewise
(*and_scc_scc): Likewise.
(*and_scc_scc_cmp): Likewise.
(*and_scc_scc_nodom): Likewise.
(*cmp_ite0, *cmp_ite1, *cmp_and, *cmp_ior): Handle Thumb2.
Testsuite Changelog would be:
2011-07-29 Jiangning Liu jiangning@arm.com
* gcc.target/arm/thumb2-cond-cmp-1.c: New. Make sure conditional
compare can be generated.
* gcc.target/arm/thumb2-cond-cmp-2.c: Likewise.
* gcc.target/arm/thumb2-cond-cmp-3.c: Likewise.
* gcc.target/arm/thumb2-cond-cmp-4.c: Likewise.
Thanks,
-Jiangning
fix_cond_cmp.patch
Description: Binary data
