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) & 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")])]
> )
