public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix for PR 61561
@ 2014-06-23 11:41 Marat Zakirov
  0 siblings, 0 replies; 9+ messages in thread
From: Marat Zakirov @ 2014-06-23 11:41 UTC (permalink / raw)
  To: gcc-patches
  Cc: 'Ramana Radhakrishnan', 'Richard Earnshaw',
	'Kyrill Tkachov', 'Slava Garbuzov',
	tetra2005, 'Marat Zakirov'

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

Hi all,

Here's my new patch for PR 61561
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61561).
Which fixes ICE appeared due to QI/HI pattern lack in arm.md for stack
pointer register. 
Reg. tested on arm-v7. 

--Marat

[-- Attachment #2: arm.diff --]
[-- Type: application/octet-stream, Size: 1985 bytes --]

gcc/ChangeLog:

2014-06-20  Marat Zakirov  <m.zakirov@samsung.com>

	PR target/61561
	* config/arm/arm.md (*movhi_insn_arch4): Handle stack pointer.
	(*movhi_bytes): Likewise.
	(*arm_movqi_insn): Likewise. 

gcc/testsuite/ChangeLog:

2014-06-20  Marat Zakirov  <m.zakirov@samsung.com>

	PR target/61561
	* gcc.dg/pr61561.c: New test.


diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 42c12c8..99290dc 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6291,7 +6291,7 @@
 ;; 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"))]
+	(match_operand:HI 1 "general_operand"      "rIk,K,r,mi"))]
   "TARGET_ARM
    && arm_arch4
    && (register_operand (operands[0], HImode)
@@ -6315,7 +6315,7 @@
 
 (define_insn "*movhi_bytes"
   [(set (match_operand:HI 0 "s_register_operand" "=r,r,r")
-	(match_operand:HI 1 "arm_rhs_operand"  "I,r,K"))]
+	(match_operand:HI 1 "arm_rhs_operand"  "I,rk,K"))]
   "TARGET_ARM"
   "@
    mov%?\\t%0, %1\\t%@ movhi
@@ -6430,7 +6430,7 @@
 
 (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"))]
+	(match_operand:QI 1 "general_operand" "rk,rk,I,Py,K,Uu,l,m,r"))]
   "TARGET_32BIT
    && (   register_operand (operands[0], QImode)
        || register_operand (operands[1], QImode))"
diff --git a/gcc/testsuite/gcc.dg/pr61561.c b/gcc/testsuite/gcc.dg/pr61561.c
new file mode 100644
index 0000000..0f4b716
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr61561.c
@@ -0,0 +1,15 @@
+/* PR c/61561.  */
+/* { dg-do assemble } */
+/* { dg-options " -w -O2" } */
+
+int dummy (int a);
+
+char a;
+short b;
+
+void mmm (void)
+{
+  char dyn[dummy (3)];
+  a = (char)&dyn[0];
+  b = (short)&dyn[0];
+}

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 20:19   ` Yuri Gribov
  2014-07-10 13:07     ` Ramana Radhakrishnan
@ 2014-07-11 15:03     ` Richard Earnshaw
  1 sibling, 0 replies; 9+ messages in thread
From: Richard Earnshaw @ 2014-07-11 15:03 UTC (permalink / raw)
  To: Yuri Gribov
  Cc: Marat Zakirov, gcc-patches, ktkachov, Gribov Yury,
	Slava Garbuzov, Marat Zakirov

On 19/06/14 21:19, Yuri Gribov wrote:
>> 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.
> 
> A general question: how should one test ARM backend patches? Is it
> enough to regtest ARM and Thumb2 on some modern Cortex? If not - what
> other variants should be tested?
> 
> -Y
> 

The answer to this question is that you have to be realistic.  It's
clearly impossible to test every permutation, but you should, ideally,
test a reasonable selection of permutations that might be affected by a
change.  Ie, if you make a change that is likely to affect thumb1 code
generation, it doesn't make sense to only test the arm or thumb2 code
generation (and vice versa).  The tricky thing here is that although
thumb1 and thumb2 sound quite similar in name, in practice thumb2 code
generation is closer to ARM than thumb1.

R.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 20:19   ` Yuri Gribov
@ 2014-07-10 13:07     ` Ramana Radhakrishnan
  2014-07-11 15:03     ` Richard Earnshaw
  1 sibling, 0 replies; 9+ messages in thread
From: Ramana Radhakrishnan @ 2014-07-10 13:07 UTC (permalink / raw)
  To: Yuri Gribov
  Cc: Richard Earnshaw, Marat Zakirov, gcc-patches, ktkachov,
	Gribov Yury, Slava Garbuzov, Marat Zakirov

On Thu, Jun 19, 2014 at 9:19 PM, Yuri Gribov <tetra2005@gmail.com> wrote:
>> 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.
>
> A general question: how should one test ARM backend patches? Is it
> enough to regtest ARM and Thumb2 on some modern Cortex? If not - what
> other variants should be tested?

We don't expect everyone to test every single variant as one wouldn't
be able to make forward progress.

1. Cross testing armv7-a / Thumb2 on qemu is acceptable.
2. If you have the resources (now with A15's) bootstrapping and
regression testing on a modern machine would be so much better.
3. Cross test on qemu for older variants that may fail- you could
consider cross testing on just the older variants. Testing on qemu
comes with it's pitfalls, the threaded tests are a joke / the asan
tests continue to randomly fail but in general it gives you a good
enough indication about how badly things maybe broken.
4. Watch out for any regressions / bug reports from folks on bugzilla
/ the mailing lists and deal with any fallout that comes promptly.

Thanks,
Ramana

>
> -Y

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 15:47 ` Richard Earnshaw
@ 2014-06-19 20:19   ` Yuri Gribov
  2014-07-10 13:07     ` Ramana Radhakrishnan
  2014-07-11 15:03     ` Richard Earnshaw
  0 siblings, 2 replies; 9+ messages in thread
From: Yuri Gribov @ 2014-06-19 20:19 UTC (permalink / raw)
  To: Richard Earnshaw
  Cc: Marat Zakirov, gcc-patches, ktkachov, Gribov Yury,
	Slava Garbuzov, Marat Zakirov

> 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.

A general question: how should one test ARM backend patches? Is it
enough to regtest ARM and Thumb2 on some modern Cortex? If not - what
other variants should be tested?

-Y

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 15:13 ` Kyrill Tkachov
  2014-06-19 15:46   ` Yuri Gribov
@ 2014-06-19 16:30   ` Ramana Radhakrishnan
  1 sibling, 0 replies; 9+ messages in thread
From: Ramana Radhakrishnan @ 2014-06-19 16:30 UTC (permalink / raw)
  To: Kyrill Tkachov, Marat Zakirov, gcc-patches
  Cc: Gribov Yury, 'Slava Garbuzov', 'Marat Zakirov',
	tetra2005, Richard Earnshaw



On 19/06/14 16:12, Kyrill Tkachov wrote:
>
> 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.

Thanks for your contribution.

However, this is *really* not the way to submit a patch and is the sort 
of patch that will make me avoid reviewing it instantly.

But given this is your first time ...... :)

Can you please explain why your fix is required ? One can understand 
what you are attempting to achieve which is adding constraints and 
alternatives for moves between sp and general purpose registers for 
HImode and QImode operands, but how did we get here in the first place ? 
My next reaction was which pass is doing this and why ?

Ah, then I go and read your bug report and your submission there in the 
description. Now I understand.

Please read - https://gcc.gnu.org/contribute.html#patches


>>
>> Reg. tested on arm15.

What's an ARM15 ? I have never seen it or even heard about it. 
Presumably you mean a Cortex-A15 and that is inferred from your comments 
in the bugzilla report.

What configuration did you test, languages ?


Now on to the patch itself.


> gcc/ChangeLog:
>
> 2014-06-19  Marat Zakirov  <m.zakirov@samsung.com>
>
> 	* config/arm/arm.md: New templates see pr61561.

See Changelog formats and comments about using PR numbers in Changelogs.

<DATE>  <NAME>  <email>

	PR target/61561
	* config/arm/arm.md (*movhi_insn_arch4): Handle stack pointer
	  (*arm_movqi_insn): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 2014-06-19  Marat Zakirov  <m.zakirov@samsung.com>
>
> 	* c-c++-common/pr61561.c: New test for pr61561.

PR target/61561
* <file_name>: New test.

>
>
> 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"

Instead replace the first source constraint with rIk rather than adding 
another alternative. You don't need a widening operation here. There is 
no need to do a zero extend here. Any widening or narrowing should be 
dealt with where required, and the store would remain a store byte and 
therefore this can be a move like the other moves between registers.

This pattern matches for arm_arch4 where uxth is not a valid 
instruction. Further more on earlier architectures this will cause an 
undefined instruction trap.



>    [(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"))])]


This should not be needed now. This is just a move_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"))]

Why do you need 2 alternatives which appear to do the same thing ? 
Instead just do a move.



>    "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" } */

Typically we put this into gcc.dg/ or if this is a test specific to ARM 
put it in gcc.target/arm with a dg-options of -O1 / -O2. In this case 
putting this in gcc.dg is probably ok.

Alternatively put this into gcc.c-torture/execute if you want this 
tortured across multiple optimization levels ?

> +
> +int dummy(int a);
> +
> +char a;
> +short b;
> +
> +void mmm (void)
> +{
> +  char dyn[ dummy(3) ];
> +  a = (char)&dyn[0];
> +  b = (short)&dyn[0];
> +}

Please submit another patch with all these changes again.

regards
Ramana

>
> CC'ing the arm maintainers...
>
> Kyrill
>
>>
>> --Marat
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 15:06 Marat Zakirov
  2014-06-19 15:13 ` Kyrill Tkachov
@ 2014-06-19 15:47 ` Richard Earnshaw
  2014-06-19 20:19   ` Yuri Gribov
  1 sibling, 1 reply; 9+ messages in thread
From: Richard Earnshaw @ 2014-06-19 15:47 UTC (permalink / raw)
  To: Marat Zakirov
  Cc: gcc-patches, ktkachov, Gribov Yury, 'Slava Garbuzov',
	'Marat Zakirov',
	tetra2005

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];
> +}
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 15:13 ` Kyrill Tkachov
@ 2014-06-19 15:46   ` Yuri Gribov
  2014-06-19 16:30   ` Ramana Radhakrishnan
  1 sibling, 0 replies; 9+ messages in thread
From: Yuri Gribov @ 2014-06-19 15:46 UTC (permalink / raw)
  To: Kyrill Tkachov
  Cc: Marat Zakirov, gcc-patches, Gribov Yury, Slava Garbuzov,
	Marat Zakirov, Ramana Radhakrishnan, Richard Earnshaw

> +                          (if_then_else (match_operand 1 "const_int_operand" "")
> +                                        (const_string "mov_imm" )
> +                                        (const_string "mov_reg"))])]

Why not just mov_reg?

> * config/arm/arm.md: New templates see pr61561.

I think you shouldn't reference PR in file change description, just
add a general comment "PR arm/61561" to each ChangeLog (see existing
ChangeLog entries).

> * c-c++-common/pr61561.c: New test for pr61561.

Likewise.

> -  "@
> +  "@

Accidental change?

-Y

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Fix for PR 61561
  2014-06-19 15:06 Marat Zakirov
@ 2014-06-19 15:13 ` Kyrill Tkachov
  2014-06-19 15:46   ` Yuri Gribov
  2014-06-19 16:30   ` Ramana Radhakrishnan
  2014-06-19 15:47 ` Richard Earnshaw
  1 sibling, 2 replies; 9+ messages in thread
From: Kyrill Tkachov @ 2014-06-19 15:13 UTC (permalink / raw)
  To: Marat Zakirov, gcc-patches
  Cc: Gribov Yury, 'Slava Garbuzov', 'Marat Zakirov',
	tetra2005, Ramana Radhakrishnan, Richard Earnshaw


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.

CC'ing the arm maintainers...

Kyrill

>
> --Marat


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Fix for PR 61561
@ 2014-06-19 15:06 Marat Zakirov
  2014-06-19 15:13 ` Kyrill Tkachov
  2014-06-19 15:47 ` Richard Earnshaw
  0 siblings, 2 replies; 9+ messages in thread
From: Marat Zakirov @ 2014-06-19 15:06 UTC (permalink / raw)
  To: gcc-patches
  Cc: ktkachov, Gribov Yury, 'Slava Garbuzov',
	'Marat Zakirov',
	tetra2005

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

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

[-- Attachment #2: arm.md.diff.diff --]
[-- Type: application/octet-stream, Size: 3792 bytes --]

gcc/ChangeLog:

2014-06-19  Marat Zakirov  <m.zakirov@samsung.com>

	* config/arm/arm.md: New templates see pr61561.

gcc/testsuite/ChangeLog:

2014-06-19  Marat Zakirov  <m.zakirov@samsung.com>

	* c-c++-common/pr61561.c: New test for pr61561.


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];
+}

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-07-11 15:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-23 11:41 [PATCH] Fix for PR 61561 Marat Zakirov
  -- strict thread matches above, loose matches on Subject: below --
2014-06-19 15:06 Marat Zakirov
2014-06-19 15:13 ` Kyrill Tkachov
2014-06-19 15:46   ` Yuri Gribov
2014-06-19 16:30   ` Ramana Radhakrishnan
2014-06-19 15:47 ` Richard Earnshaw
2014-06-19 20:19   ` Yuri Gribov
2014-07-10 13:07     ` Ramana Radhakrishnan
2014-07-11 15:03     ` Richard Earnshaw

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).