Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 19 August 2011 11:06, Ramana Radhakrishnan
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
* 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
* 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
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 * 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 * 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 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>
> it
> cmn reg, <8bitconstant>
>
> Length = 6 bytes
>
> or even with
>cmp reg, reg
>it
>cmn 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_do
Re: [PATCH, ARM] Generate conditional compares in Thumb2 state
On 10 August 2011 09:20, Jiangning Liu 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>
it
cmn reg, <8bitconstant>
Length = 6 bytes
or even with
cmp reg, reg
it
cmn 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
generation. Conditional instructions in GCC in RTL form usually have
a cond-exec associated with them. The way this works is to look for
sequences of cond-execs and inspect their conditions to merge
them together later. Look at thumb2_final_prescan_insn in arm.c for
more information. Mostly in the cases where we have such instructions
especially where we have conditional compares being generated.
My suggestion in the previous mail about splitting this into cond-exec form
instructions would help without changing too much of the current
infrastructure. I am happy to treat that as a follow-up set of patches.
cheers
Ramana
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 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 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
> 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 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
On 3 August 2011 08:48, Jiangning Liu 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
