> > 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) & 0xffff0000) == 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
