Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 19/11/14 09:29, Yangfei (Felix) wrote: Sorry for missing the point. It seems to me that 't2' here will conflict with condition of the pattern *movhi_insn_arch4: TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) || register_operand (operands[1], HImode)) #define TARGET_ARM (! TARGET_THUMB) /* 32-bit Thumb-2 code. */ #define TARGET_THUMB2 (TARGET_THUMB arm_arch_thumb2) Bah, Indeed ! - I misremembered the t2 there, my mistake. Yes you are right there, but what I'd like you to do is to use that mechanism rather than putting all this logic in the predicate. So, I'd prefer you to add a v6t2 to the values for the arch attribute, don't forget to update the comments above. and in arch_enabled you need to enforce this with (and (eq_attr arch v6t2) (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) (const_string yes) And in the pattern use v6t2 ... arm_arch_thumb2 implies that this is at the architecture level of v6t2. Therefore TARGET_ARM arm_arch_thumb2 implies ARM state. Hi Ramana, Thank you for your suggestions. I rebased the patch on the latest trunk and updated it accordingly. As this patch will not work for architectures older than armv6t2, I also prefer Thomas's patch to fix for them. I am currently performing test for this patch. Assuming no issues pops up, OK for the trunk? And is it necessary to backport this patch to the 4.8 4.9 branches? I've applied the following as obvious after Kugan mentioned on IRC this morning noticing a movwne r0, #-32768. Obviously this won't be accepted as is by the assembler and we should be using the %L character. Applied to trunk as obvious. Felix, How did you test this patch ? regards Ramana 2014-11-20 Ramana Radhakrishnan ramana.radhakrish...@arm.com PR target/59593 * config/arm/arm.md (*movhi_insn): Use right formatting for immediate. Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 217717) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2014-11-19 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + + PR target/59593 + * config/arm/arm.md (define_attr arch): Add v6t2. + (define_attr arch_enabled): Add test for the above. + (*movhi_insn_arch4): Add new alternative. + 2014-11-18 Felix Yang felix.y...@huawei.com * config/aarch64/aarch64.c (doloop_end): New pattern. Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 217717) +++ gcc/config/arm/arm.md (working copy) @@ -125,9 +125,10 @@ ; This can be a for ARM, t for either of the Thumbs, 32 for ; TARGET_32BIT, t1 or t2 to specify a specific Thumb mode. v6 ; for ARM or Thumb-2 with arm_arch6, and nov6 for ARM without -; arm_arch6. This attribute is used to compute attribute enabled, -; use type any to enable an alternative in all cases. -(define_attr arch any,a,t,32,t1,t2,v6,nov6,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3 +; arm_arch6. v6t2 for Thumb-2 with arm_arch6. This attribute is +; used to compute attribute enabled, use type any to enable an +; alternative in all cases. +(define_attr arch any,a,t,32,t1,t2,v6,nov6,v6t2,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3 (const_string any)) (define_attr arch_enabled no,yes @@ -162,6 +163,10 @@ (match_test TARGET_32BIT !arm_arch6)) (const_string yes) +(and (eq_attr arch v6t2) + (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) +(const_string yes) + (and (eq_attr arch avoid_neon_for_64bits) (match_test TARGET_NEON) (not (match_test TARGET_PREFER_NEON_64BITS))) @@ -6288,8 +6293,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 general_operand rIk,K,n,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6297,16 +6302,19 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) + (set_attr arch *,*,v6t2,*,*) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm )
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 19/11/14 09:29, Yangfei (Felix) wrote:
Sorry for missing the point. It seems to me that 't2' here will
conflict with
condition of the pattern *movhi_insn_arch4:
TARGET_ARM
arm_arch4
(register_operand (operands[0], HImode)
|| register_operand (operands[1], HImode))
#define TARGET_ARM (! TARGET_THUMB)
/* 32-bit Thumb-2 code. */
#define TARGET_THUMB2 (TARGET_THUMB
arm_arch_thumb2)
Bah, Indeed ! - I misremembered the t2 there, my mistake.
Yes you are right there, but what I'd like you to do is to use that
mechanism rather than putting all this logic in the predicate.
So, I'd prefer you to add a v6t2 to the values for the arch
attribute, don't forget to update the comments above.
and in arch_enabled you need to enforce this with
(and (eq_attr arch v6t2)
(match_test TARGET_32BIT arm_arch6
arm_arch_thumb2))
(const_string yes)
And in the pattern use v6t2 ...
arm_arch_thumb2 implies that this is at the architecture level of v6t2.
Therefore TARGET_ARM arm_arch_thumb2 implies ARM state.
Hi Ramana,
Thank you for your suggestions. I rebased the patch on the latest
trunk
and updated it accordingly.
As this patch will not work for architectures older than armv6t2, I
also
prefer Thomas's patch to fix for them.
I am currently performing test for this patch. Assuming no issues pops
up, OK for the trunk?
And is it necessary to backport this patch to the 4.8 4.9 branches?
I've applied the following as obvious after Kugan mentioned on IRC this
morning
noticing a movwne r0, #-32768. Obviously this won't be accepted as is by the
assembler and we should be using the %L character. Applied to trunk as
obvious.
Felix, How did you test this patch ?
regards
Ramana
I regtested the patch for arm-eabi-gcc/g++ big-endian with qemu. The test
result is OK. That's strange ...
This issue can be reproduced by the following testcase. Thanks for fixing it.
#include stdio.h
unsigned short v = 0x5678;
int i;
int j = 0;
int *ptr = j;
int func()
{
for (i = 0; i 1; ++i)
{
*ptr = -1;
v = 0xF234;
}
return v;
}
2014-11-20 Ramana Radhakrishnan ramana.radhakrish...@arm.com
PR target/59593
* config/arm/arm.md (*movhi_insn): Use right formatting
for immediate.
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 19/11/14 09:29, Yangfei (Felix) wrote:
Sorry for missing the point. It seems to me that 't2' here will
conflict with
condition of the pattern *movhi_insn_arch4:
TARGET_ARM
arm_arch4
(register_operand (operands[0], HImode)
|| register_operand (operands[1], HImode))
#define TARGET_ARM (! TARGET_THUMB)
/* 32-bit Thumb-2 code. */
#define TARGET_THUMB2 (TARGET_THUMB
arm_arch_thumb2)
Bah, Indeed ! - I misremembered the t2 there, my mistake.
Yes you are right there, but what I'd like you to do is to use that
mechanism rather than putting all this logic in the predicate.
So, I'd prefer you to add a v6t2 to the values for the arch
attribute, don't forget to update the comments above.
and in arch_enabled you need to enforce this with
(and (eq_attr arch v6t2)
(match_test TARGET_32BIT arm_arch6
arm_arch_thumb2))
(const_string yes)
And in the pattern use v6t2 ...
arm_arch_thumb2 implies that this is at the architecture level of v6t2.
Therefore TARGET_ARM arm_arch_thumb2 implies ARM state.
Hi Ramana,
Thank you for your suggestions. I rebased the patch on the
latest trunk
and updated it accordingly.
As this patch will not work for architectures older than
armv6t2, I also
prefer Thomas's patch to fix for them.
I am currently performing test for this patch. Assuming no
issues pops
up, OK for the trunk?
And is it necessary to backport this patch to the 4.8 4.9 branches?
I've applied the following as obvious after Kugan mentioned on IRC
this morning noticing a movwne r0, #-32768. Obviously this won't be
accepted as is by the assembler and we should be using the %L character.
Applied to trunk as obvious.
Felix, How did you test this patch ?
regards
Ramana
I regtested the patch for arm-eabi-gcc/g++ big-endian with qemu. The test
result is OK. That's strange ...
This issue can be reproduced by the following testcase. Thanks for fixing it.
#include stdio.h
unsigned short v = 0x5678;
int i;
int j = 0;
int *ptr = j;
int func()
{
for (i = 0; i 1; ++i)
{
*ptr = -1;
v = 0xF234;
}
return v;
}
And the architecture level is set to armv7-a by default when testing.
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
Sorry for missing the point. It seems to me that 't2' here will conflict with condition of the pattern *movhi_insn_arch4: TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) || register_operand (operands[1], HImode)) #define TARGET_ARM (! TARGET_THUMB) /* 32-bit Thumb-2 code. */ #define TARGET_THUMB2 (TARGET_THUMB arm_arch_thumb2) Bah, Indeed ! - I misremembered the t2 there, my mistake. Yes you are right there, but what I'd like you to do is to use that mechanism rather than putting all this logic in the predicate. So, I'd prefer you to add a v6t2 to the values for the arch attribute, don't forget to update the comments above. and in arch_enabled you need to enforce this with (and (eq_attr arch v6t2) (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) (const_string yes) And in the pattern use v6t2 ... arm_arch_thumb2 implies that this is at the architecture level of v6t2. Therefore TARGET_ARM arm_arch_thumb2 implies ARM state. Hi Ramana, Thank you for your suggestions. I rebased the patch on the latest trunk and updated it accordingly. As this patch will not work for architectures older than armv6t2, I also prefer Thomas's patch to fix for them. I am currently performing test for this patch. Assuming no issues pops up, OK for the trunk? And is it necessary to backport this patch to the 4.8 4.9 branches? Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 217717) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2014-11-19 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + + PR target/59593 + * config/arm/arm.md (define_attr arch): Add v6t2. + (define_attr arch_enabled): Add test for the above. + (*movhi_insn_arch4): Add new alternative. + 2014-11-18 Felix Yang felix.y...@huawei.com * config/aarch64/aarch64.c (doloop_end): New pattern. Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 217717) +++ gcc/config/arm/arm.md (working copy) @@ -125,9 +125,10 @@ ; This can be a for ARM, t for either of the Thumbs, 32 for ; TARGET_32BIT, t1 or t2 to specify a specific Thumb mode. v6 ; for ARM or Thumb-2 with arm_arch6, and nov6 for ARM without -; arm_arch6. This attribute is used to compute attribute enabled, -; use type any to enable an alternative in all cases. -(define_attr arch any,a,t,32,t1,t2,v6,nov6,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3 +; arm_arch6. v6t2 for Thumb-2 with arm_arch6. This attribute is +; used to compute attribute enabled, use type any to enable an +; alternative in all cases. +(define_attr arch any,a,t,32,t1,t2,v6,nov6,v6t2,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3 (const_string any)) (define_attr arch_enabled no,yes @@ -162,6 +163,10 @@ (match_test TARGET_32BIT !arm_arch6)) (const_string yes) +(and (eq_attr arch v6t2) + (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) +(const_string yes) + (and (eq_attr arch avoid_neon_for_64bits) (match_test TARGET_NEON) (not (match_test TARGET_PREFER_NEON_64BITS))) @@ -6288,8 +6293,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 general_operand rIk,K,n,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6297,16 +6302,19 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) + (set_attr arch *,*,v6t2,*,*) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string load1)])] ) arm-patch-v3.diff Description: arm-patch-v3.diff
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 19/11/14 09:29, Yangfei (Felix) wrote: Sorry for missing the point. It seems to me that 't2' here will conflict with condition of the pattern *movhi_insn_arch4: TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) || register_operand (operands[1], HImode)) #define TARGET_ARM (! TARGET_THUMB) /* 32-bit Thumb-2 code. */ #define TARGET_THUMB2 (TARGET_THUMB arm_arch_thumb2) Bah, Indeed ! - I misremembered the t2 there, my mistake. Yes you are right there, but what I'd like you to do is to use that mechanism rather than putting all this logic in the predicate. So, I'd prefer you to add a v6t2 to the values for the arch attribute, don't forget to update the comments above. and in arch_enabled you need to enforce this with (and (eq_attr arch v6t2) (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) (const_string yes) And in the pattern use v6t2 ... arm_arch_thumb2 implies that this is at the architecture level of v6t2. Therefore TARGET_ARM arm_arch_thumb2 implies ARM state. Hi Ramana, Thank you for your suggestions. I rebased the patch on the latest trunk and updated it accordingly. As this patch will not work for architectures older than armv6t2, I also prefer Thomas's patch to fix for them. I am currently performing test for this patch. Assuming no issues pops up, OK for the trunk? And is it necessary to backport this patch to the 4.8 4.9 branches? This is OK for trunk if no regressions - please let it bake for a few days on trunk to let auto-testers catch up. This is OK for 4.8 / 4.9 after that and please test your backport. regards Ramana Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 217717) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,11 @@ +2014-11-19 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + + PR target/59593 + * config/arm/arm.md (define_attr arch): Add v6t2. + (define_attr arch_enabled): Add test for the above. + (*movhi_insn_arch4): Add new alternative. + 2014-11-18 Felix Yang felix.y...@huawei.com * config/aarch64/aarch64.c (doloop_end): New pattern. Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 217717) +++ gcc/config/arm/arm.md (working copy) @@ -125,9 +125,10 @@ ; This can be a for ARM, t for either of the Thumbs, 32 for ; TARGET_32BIT, t1 or t2 to specify a specific Thumb mode. v6 ; for ARM or Thumb-2 with arm_arch6, and nov6 for ARM without -; arm_arch6. This attribute is used to compute attribute enabled, -; use type any to enable an alternative in all cases. -(define_attr arch any,a,t,32,t1,t2,v6,nov6,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3 +; arm_arch6. v6t2 for Thumb-2 with arm_arch6. This attribute is +; used to compute attribute enabled, use type any to enable an +; alternative in all cases. +(define_attr arch any,a,t,32,t1,t2,v6,nov6,v6t2,neon_for_64bits,avoid_neon_for_64bits,iwmmxt,iwmmxt2,armv6_or_vfpv3 (const_string any)) (define_attr arch_enabled no,yes @@ -162,6 +163,10 @@ (match_test TARGET_32BIT !arm_arch6)) (const_string yes) +(and (eq_attr arch v6t2) + (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) +(const_string yes) + (and (eq_attr arch avoid_neon_for_64bits) (match_test TARGET_NEON) (not (match_test TARGET_PREFER_NEON_64BITS))) @@ -6288,8 +6293,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 general_operand rIk,K,n,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6297,16 +6302,19 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) + (set_attr arch *,*,v6t2,*,*) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string load1)])] )
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 06/11/14 08:35, Yangfei (Felix) wrote: The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Actually, we fix the bug by excluding the const values which cannot be handled. The patch still works even without the adding of movw here. The new movw alternative here is just an small code optimization for certain arch. We can handle consts by movw instead of ldrh and this better for performance. We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is that this pattern can always move consts using movw. Please fix the PR number in your final commit with PR 59593. Index: gcc/config/arm/predicates.md === --- gcc/config/arm/predicates.md(revision 216838) +++ gcc/config/arm/predicates.md(working copy) @@ -144,6 +144,11 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +216,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + I think you don't need any of these predicates. (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] Use `n' instead of `j' - movw can handle all of the numerical constants for HImode values. And the predicate can remain general_operand. TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6294,16 +6294,18 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string load1)])] ) Look at the set_attr arch alternative and set this to t2 for the movw alternative. I would expect that to be enough to sort this out instead of adding all this code. I don't think your patch or the one with my modifications fixes the issue completely for architectures that do not have movw / movt instructions and I would expect that one would still need Thomas's patch to create const table entries of the right size. Otherwise this is OK. regards Ramana --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + I'm assuming you have copyright assignments sorted. Yes, both my employer(Huawei) and I have signed copyright assignments with FSF. +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (ior (match_code high) I don't see why you need to check for high here ? Yeah, I double checked and found that it's not necessary here. Thanks for pointing this out. Attached please find the updated patch. Reg-tested for armeb-none-eabi. OK for the trunk? Index: gcc/ChangeLog
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 06/11/14 08:35, Yangfei (Felix) wrote: The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Actually, we fix the bug by excluding the const values which cannot be handled. The patch still works even without the adding of movw here. The new movw alternative here is just an small code optimization for certain arch. We can handle consts by movw instead of ldrh and this better for performance. We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is that this pattern can always move consts using movw. Please fix the PR number in your final commit with PR 59593. Index: gcc/config/arm/predicates.md = == --- gcc/config/arm/predicates.md(revision 216838) +++ gcc/config/arm/predicates.md(working copy) @@ -144,6 +144,11 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +216,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + I think you don't need any of these predicates. (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md = == --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] Use `n' instead of `j' - movw can handle all of the numerical constants for HImode values. And the predicate can remain general_operand. Hello Ramana, We need to make sure that movw is only used for architectures which satisfy arm_arch_thumb2 as indicated in the following predicate added: +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 I am modifying the predicate in order to fix issue for other older architectures. It seems we can't achieve this by simply using 'n' instead of 'j' here, right? Thanks.
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 18/11/14 11:02, Yangfei (Felix) wrote: On 06/11/14 08:35, Yangfei (Felix) wrote: The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Actually, we fix the bug by excluding the const values which cannot be handled. The patch still works even without the adding of movw here. The new movw alternative here is just an small code optimization for certain arch. We can handle consts by movw instead of ldrh and this better for performance. We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is that this pattern can always move consts using movw. Please fix the PR number in your final commit with PR 59593. Index: gcc/config/arm/predicates.md = == --- gcc/config/arm/predicates.md(revision 216838) +++ gcc/config/arm/predicates.md(working copy) @@ -144,6 +144,11 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +216,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + I think you don't need any of these predicates. (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md = == --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] Use `n' instead of `j' - movw can handle all of the numerical constants for HImode values. And the predicate can remain general_operand. Did you read my comment about set_attr arch further down in the thread ? Look at the set_attr arch alternative and set this to t2 for the movw alternative. I would expect that to be enough to sort this out instead of adding all this code. Ramana Hello Ramana, We need to make sure that movw is only used for architectures which satisfy arm_arch_thumb2 as indicated in the following predicate added: +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 I am modifying the predicate in order to fix issue for other older architectures. It seems we can't achieve this by simply using 'n' instead of 'j' here, right? Thanks.
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 18/11/14 11:02, Yangfei (Felix) wrote: On 06/11/14 08:35, Yangfei (Felix) wrote: The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Actually, we fix the bug by excluding the const values which cannot be handled. The patch still works even without the adding of movw here. The new movw alternative here is just an small code optimization for certain arch. We can handle consts by movw instead of ldrh and this better for performance. We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is that this pattern can always move consts using movw. Please fix the PR number in your final commit with PR 59593. Index: gcc/config/arm/predicates.md = == --- gcc/config/arm/predicates.md (revision 216838) +++ gcc/config/arm/predicates.md (working copy) @@ -144,6 +144,11 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +216,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + I think you don't need any of these predicates. (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md = == --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] Use `n' instead of `j' - movw can handle all of the numerical constants for HImode values. And the predicate can remain general_operand. Did you read my comment about set_attr arch further down in the thread ? Look at the set_attr arch alternative and set this to t2 for the movw alternative. I would expect that to be enough to sort this out instead of adding all this code. Sorry for missing the point. It seems to me that 't2' here will conflict with condition of the pattern *movhi_insn_arch4: TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) || register_operand (operands[1], HImode)) #define TARGET_ARM (! TARGET_THUMB) /* 32-bit Thumb-2 code. */ #define TARGET_THUMB2 (TARGET_THUMB arm_arch_thumb2) Right? Thanks.
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
Sorry for missing the point. It seems to me that 't2' here will conflict with condition of the pattern *movhi_insn_arch4: TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) || register_operand (operands[1], HImode)) #define TARGET_ARM (! TARGET_THUMB) /* 32-bit Thumb-2 code. */ #define TARGET_THUMB2 (TARGET_THUMB arm_arch_thumb2) Bah, Indeed ! - I misremembered the t2 there, my mistake. Yes you are right there, but what I'd like you to do is to use that mechanism rather than putting all this logic in the predicate. So, I'd prefer you to add a v6t2 to the values for the arch attribute, don't forget to update the comments above. and in arch_enabled you need to enforce this with (and (eq_attr arch v6t2) (match_test TARGET_32BIT arm_arch6 arm_arch_thumb2)) (const_string yes) And in the pattern use v6t2 ... arm_arch_thumb2 implies that this is at the architecture level of v6t2. Therefore TARGET_ARM arm_arch_thumb2 implies ARM state. regards Ramana Right? Thanks.
[PING][PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
Hello, Any comments on this patch? Thanks. The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Actually, we fix the bug by excluding the const values which cannot be handled. The patch still works even without the adding of movw here. The new movw alternative here is just an small code optimization for certain arch. We can handle consts by movw instead of ldrh and this better for performance. We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is that this pattern can always move consts using movw. --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + I'm assuming you have copyright assignments sorted. Yes, both my employer(Huawei) and I have signed copyright assignments with FSF. +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (ior (match_code high) I don't see why you need to check for high here ? Yeah, I double checked and found that it's not necessary here. Thanks for pointing this out. Attached please find the updated patch. Reg-tested for armeb-none-eabi. OK for the trunk? Index: gcc/ChangeLog = == --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + + PR target/63742 + * config/arm/predicates.md (arm_hi_operand): New predicate. + (arm_movw_immediate_operand): Similarly. + * config/arm/arm.md (*movhi_insn_arch4): Use arm_hi_operand instead of + general_operand and add movw to the output template. + 2014-10-29 Richard Sandiford richard.sandif...@arm.com * addresses.h, alias.c, asan.c, auto-inc-dec.c, bt-load.c, builtins.c, Index: gcc/config/arm/predicates.md = == --- gcc/config/arm/predicates.md (revision 216838) +++ gcc/config/arm/predicates.md (working copy) @@ -144,6 +144,11 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +216,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md = == --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6294,16 +6294,18 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Actually, we fix the bug by excluding the const values which cannot be handled. The patch still works even without the adding of movw here. The new movw alternative here is just an small code optimization for certain arch. We can handle consts by movw instead of ldrh and this better for performance. We find that this bug is not reproducible for *thumb2_movhi_insn. The reason is that this pattern can always move consts using movw. --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + I'm assuming you have copyright assignments sorted. Yes, both my employer(Huawei) and I have signed copyright assignments with FSF. +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (ior (match_code high) I don't see why you need to check for high here ? Yeah, I double checked and found that it's not necessary here. Thanks for pointing this out. Attached please find the updated patch. Reg-tested for armeb-none-eabi. OK for the trunk? Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + + PR target/63742 + * config/arm/predicates.md (arm_hi_operand): New predicate. + (arm_movw_immediate_operand): Similarly. + * config/arm/arm.md (*movhi_insn_arch4): Use arm_hi_operand instead of + general_operand and add movw to the output template. + 2014-10-29 Richard Sandiford richard.sandif...@arm.com * addresses.h, alias.c, asan.c, auto-inc-dec.c, bt-load.c, builtins.c, Index: gcc/config/arm/predicates.md === --- gcc/config/arm/predicates.md(revision 216838) +++ gcc/config/arm/predicates.md(working copy) @@ -144,6 +144,11 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0 + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +216,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6294,16 +6294,18 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string load1)])] ) movhi_insn_arch4-fix-v2.patch Description: movhi_insn_arch4-fix-v2.patch
Re: [PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
On 05/11/14 07:09, Yangfei (Felix) wrote: Hi, This patch fixes PR63742 by improving arm *movhi_insn_arch4 pattern to make it works under big-endian. The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? So, doesn't that makes the bug latent for architectures older than armv6t2 and big endian and only fixed this in ARM state ? I'd prefer a complete solution please. What about *thumb2_movhi_insn in thumb2.md ? Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + I'm assuming you have copyright assignments sorted. + PR target/63742 + * config/arm/predicates.md (arm_hi_operand): New predicate. + (arm_movw_immediate_operand): Similarly. + * config/arm/arm.md (*movhi_insn_arch4): Use arm_hi_operand instead of + general_operand and add movw to the output template. + 2014-10-29 Richard Sandiford richard.sandif...@arm.com * addresses.h, alias.c, asan.c, auto-inc-dec.c, bt-load.c, builtins.c, Index: gcc/config/arm/predicates.md === --- gcc/config/arm/predicates.md(revision 216838) +++ gcc/config/arm/predicates.md(working copy) @@ -144,6 +144,12 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (ior (match_code high) I don't see why you need to check for high here ? +(and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0) + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +217,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6294,16 +6294,18 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string load1)])] ) Ramana
[PATCH, PR63742][ARM] Fix arm *movhi_insn_arch4 pattern for big-endian
Hi, This patch fixes PR63742 by improving arm *movhi_insn_arch4 pattern to make it works under big-endian. The idea is simple: Use movw for certain const source operand instead of ldrh. And exclude the const values which cannot be handled by mov/mvn/movw. I am doing regression test for this patch. Assuming no issue pops up, OK for trunk? Index: gcc/ChangeLog === --- gcc/ChangeLog (revision 216838) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,12 @@ +2014-11-05 Felix Yang felix.y...@huawei.com + Shanyao Chen chenshan...@huawei.com + + PR target/63742 + * config/arm/predicates.md (arm_hi_operand): New predicate. + (arm_movw_immediate_operand): Similarly. + * config/arm/arm.md (*movhi_insn_arch4): Use arm_hi_operand instead of + general_operand and add movw to the output template. + 2014-10-29 Richard Sandiford richard.sandif...@arm.com * addresses.h, alias.c, asan.c, auto-inc-dec.c, bt-load.c, builtins.c, Index: gcc/config/arm/predicates.md === --- gcc/config/arm/predicates.md(revision 216838) +++ gcc/config/arm/predicates.md(working copy) @@ -144,6 +144,12 @@ (and (match_code const_int) (match_test INTVAL (op) == 0))) +(define_predicate arm_movw_immediate_operand + (and (match_test TARGET_32BIT arm_arch_thumb2) + (ior (match_code high) +(and (match_code const_int) + (match_test (INTVAL (op) 0x) == 0) + ;; Something valid on the RHS of an ARM data-processing instruction (define_predicate arm_rhs_operand (ior (match_operand 0 s_register_operand) @@ -211,6 +217,11 @@ (ior (match_operand 0 arm_rhs_operand) (match_operand 0 arm_not_immediate_operand))) +(define_predicate arm_hi_operand + (ior (match_operand 0 arm_rhsm_operand) + (ior (match_operand 0 arm_not_immediate_operand) +(match_operand 0 arm_movw_immediate_operand + (define_predicate arm_di_operand (ior (match_operand 0 s_register_operand) (match_operand 0 arm_immediate_di_operand))) Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 216838) +++ gcc/config/arm/arm.md (working copy) @@ -6285,8 +6285,8 @@ ;; Pattern to recognize insn generated default case above (define_insn *movhi_insn_arch4 - [(set (match_operand:HI 0 nonimmediate_operand =r,r,m,r) - (match_operand:HI 1 general_operand rIk,K,r,mi))] + [(set (match_operand:HI 0 nonimmediate_operand =r,r,r,m,r) + (match_operand:HI 1 arm_hi_operand rIk,K,j,r,mi))] TARGET_ARM arm_arch4 (register_operand (operands[0], HImode) @@ -6294,16 +6294,18 @@ @ mov%?\\t%0, %1\\t%@ movhi mvn%?\\t%0, #%B1\\t%@ movhi + movw%?\\t%0, %1\\t%@ movhi str%(h%)\\t%1, %0\\t%@ movhi ldr%(h%)\\t%0, %1\\t%@ movhi [(set_attr predicable yes) - (set_attr pool_range *,*,*,256) - (set_attr neg_pool_range *,*,*,244) + (set_attr pool_range *,*,*,*,256) + (set_attr neg_pool_range *,*,*,*,244) (set_attr_alternative type [(if_then_else (match_operand 1 const_int_operand ) (const_string mov_imm ) (const_string mov_reg)) (const_string mvn_imm) + (const_string mov_imm) (const_string store1) (const_string load1)])] ) movhi_insn_arch4-fix-v1.patch Description: movhi_insn_arch4-fix-v1.patch
