Re: [PATCH] Fix for PR 61561

[Richard Earnshaw <rearnsha at arm dot com>]

On 19/06/14 16:05, Marat Zakirov wrote:
> Hi all,
> 
> Here's a patch for PR 61561
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561). 
> 
> It fixes ICE.
> 
> Reg. tested on arm15.
> 
> --Marat
> 
> 
> arm.md.diff.diff
> 
> 
> gcc/ChangeLog:
> 
> 2014-06-19  Marat Zakirov  <m.zakirov@samsung.com>
> 
> 	* config/arm/arm.md: New templates see pr61561.
> 

ChangeLog entries should list the names of the patterns changed.

> gcc/testsuite/ChangeLog:
> 
> 2014-06-19  Marat Zakirov  <m.zakirov@samsung.com>
> 
> 	* c-c++-common/pr61561.c: New test for pr61561.
> 

Not OK.  I don't see why you need to zero out the top bits.

Firstly, it shouldn't be necessary to have a new alternative; I see no
reason why these can't use the MOV instruction in alternative 0 (just
change rI to rkI).

Secondly, uxt[bh] are ARMv6 and later, but this pattern needs to work
for armv4 and later.

Thirdly, we also need to fix movhi_bytes (for pre-v4) thumb2_movhi_insn
(for thumb2) and, quite possibly, thumb1_movhi_insn (for thumb1).  There
may well be additional changes for movqi variants as well.

R.

> 
> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index 42c12c8..7ed8abc 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -6290,27 +6290,31 @@
>  
>  ;; 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"      "rI,K,r,mi"))]
> +  [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r,m,r,r")
> +	(match_operand:HI 1 "general_operand"      "rI,K,r,mi,k"))]
>    "TARGET_ARM
>     && arm_arch4
>     && (register_operand (operands[0], HImode)
>         || register_operand (operands[1], HImode))"
> -  "@
> +  "@   
>     mov%?\\t%0, %1\\t%@ movhi
>     mvn%?\\t%0, #%B1\\t%@ movhi
>     str%(h%)\\t%1, %0\\t%@ movhi
> -   ldr%(h%)\\t%0, %1\\t%@ movhi"
> +   ldr%(h%)\\t%0, %1\\t%@ movhi
> +   uxth%?\\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 "store1")
> -                          (const_string "load1")])]
> +                          (const_string "load1")
> +                          (if_then_else (match_operand 1 "const_int_operand" "")
> +                                        (const_string "mov_imm" )
> +                                        (const_string "mov_reg"))])]
>  )
>  
>  (define_insn "*movhi_bytes"
> @@ -6429,8 +6433,8 @@
>  )
>  
>  (define_insn "*arm_movqi_insn"
> -  [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m")
> -	(match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r"))]
> +  [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,l,r,l,Uu,r,m,r,r")
> +	(match_operand:QI 1 "general_operand" "r,r,I,Py,K,Uu,l,m,r,k,k"))]
>    "TARGET_32BIT
>     && (   register_operand (operands[0], QImode)
>         || register_operand (operands[1], QImode))"
> @@ -6443,12 +6447,14 @@
>     ldr%(b%)\\t%0, %1
>     str%(b%)\\t%1, %0
>     ldr%(b%)\\t%0, %1
> -   str%(b%)\\t%1, %0"
> -  [(set_attr "type" "mov_reg,mov_reg,mov_imm,mov_imm,mvn_imm,load1,store1,load1,store1")
> +   str%(b%)\\t%1, %0
> +   uxtb%?\\t%0, %1
> +   uxtb%?\\t%0, %1"
> +  [(set_attr "type" "mov_reg,mov_reg,mov_imm,mov_imm,mvn_imm,load1,store1,load1,store1,mov_reg,mov_reg")
>     (set_attr "predicable" "yes")
> -   (set_attr "predicable_short_it" "yes,yes,yes,no,no,no,no,no,no")
> -   (set_attr "arch" "t2,any,any,t2,any,t2,t2,any,any")
> -   (set_attr "length" "2,4,4,2,4,2,2,4,4")]
> +   (set_attr "predicable_short_it" "yes,yes,yes,no,no,no,no,no,no,no,no")
> +   (set_attr "arch" "t2,any,any,t2,any,t2,t2,any,any,any,t2")
> +   (set_attr "length" "2,4,4,2,4,2,2,4,4,4,2")]
>  )
>  
>  ;; HFmode moves
> diff --git a/gcc/testsuite/c-c++-common/pr61561.c b/gcc/testsuite/c-c++-common/pr61561.c
> new file mode 100644
> index 0000000..0f4b716
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr61561.c
> @@ -0,0 +1,15 @@
> +/* PR c/61561 */
> +/* { dg-do assemble } */
> +/* { dg-options " -w" } */
> +
> +int dummy(int a);
> +
> +char a;
> +short b;
> +
> +void mmm (void)
> +{
> +  char dyn[ dummy(3) ];
> +  a = (char)&dyn[0];
> +  b = (short)&dyn[0];
> +}
>