[PATCH][ARM] Do not expand movmisalign pattern if not in 32-bit mode

[PATCH][ARM] Do not expand movmisalign pattern if not in 32-bit mode

[Kyrill Tkachov]

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

Hi all,

The attached testcase ICEs when compiled with -march=armv6k -mthumb -Os or any march
for which -mthumb gives Thumb1:
  error: unrecognizable insn:
  }
  ^
(insn 13 12 14 5 (set (reg:SI 116 [ x ])
         (unspec:SI [
                 (mem:SI (reg/v/f:SI 112 [ s ]) [0 MEM[(unsigned char *)s_1(D)]+0 S4 A8])
             ] UNSPEC_UNALIGNED_LOAD)) besttry.c:9 -1
      (nil))

The problem is that the expands a movmisalign pattern but the resulting unaligned loads don't
match any define_insn because they are gated on unaligned_access && TARGET_32BIT.
The unaligned_access expander is gated only on unaligned_access.

This small patch fixes the issue by turning off unaligned_access if TARGET_32BIT is not true.
We can then remove TARGET_32BIT from the unaligned load/store patterns conditions as a cleanup.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

2015-11-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/arm.c (arm_option_override): Require TARGET_32BIT
     for unaligned_access.
     * config/arm/arm.md (unaligned_loadsi): Remove redundant TARGET_32BIT
     from matching condition.
     (unaligned_loadhis): Likewise.
     (unaligned_loadhiu): Likewise.
     (unaligned_storesi): Likewise.
     (unaligned_storehi): Likewise.

2015-11-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * gcc.target/arm/armv6-unaligned-load-ice.c: New test.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: arm-armv6-unaligned-ice.patch --]
[-- Type: text/x-patch; name=arm-armv6-unaligned-ice.patch, Size: 3798 bytes --]

commit 3b1e68a9f7fadeeb6d7f201ce2291bf2286a4d63
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Tue Nov 10 13:48:17 2015 +0000

    [ARM] Do not expand movmisalign pattern if not in 32-bit mode

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6a0994e..4708a12 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3436,7 +3436,8 @@ arm_option_override (void)
     }
 
   /* Enable -munaligned-access by default for
-     - all ARMv6 architecture-based processors
+     - all ARMv6 architecture-based processors when compiling for a 32-bit ISA
+     i.e. Thumb2 and ARM state only.
      - ARMv7-A, ARMv7-R, and ARMv7-M architecture-based processors.
      - ARMv8 architecture-base processors.
 
@@ -3446,7 +3447,7 @@ arm_option_override (void)
 
   if (unaligned_access == 2)
     {
-      if (arm_arch6 && (arm_arch_notm || arm_arch7))
+      if (TARGET_32BIT && arm_arch6 && (arm_arch_notm || arm_arch7))
 	unaligned_access = 1;
       else
 	unaligned_access = 0;
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index ab48873..090a287 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -4266,7 +4266,7 @@ (define_insn "unaligned_loadsi"
   [(set (match_operand:SI 0 "s_register_operand" "=l,r")
 	(unspec:SI [(match_operand:SI 1 "memory_operand" "Uw,m")]
 		   UNSPEC_UNALIGNED_LOAD))]
-  "unaligned_access && TARGET_32BIT"
+  "unaligned_access"
   "ldr%?\t%0, %1\t@ unaligned"
   [(set_attr "arch" "t2,any")
    (set_attr "length" "2,4")
@@ -4279,7 +4279,7 @@ (define_insn "unaligned_loadhis"
 	(sign_extend:SI
 	  (unspec:HI [(match_operand:HI 1 "memory_operand" "Uw,Uh")]
 		     UNSPEC_UNALIGNED_LOAD)))]
-  "unaligned_access && TARGET_32BIT"
+  "unaligned_access"
   "ldrsh%?\t%0, %1\t@ unaligned"
   [(set_attr "arch" "t2,any")
    (set_attr "length" "2,4")
@@ -4292,7 +4292,7 @@ (define_insn "unaligned_loadhiu"
 	(zero_extend:SI
 	  (unspec:HI [(match_operand:HI 1 "memory_operand" "Uw,m")]
 		     UNSPEC_UNALIGNED_LOAD)))]
-  "unaligned_access && TARGET_32BIT"
+  "unaligned_access"
   "ldrh%?\t%0, %1\t@ unaligned"
   [(set_attr "arch" "t2,any")
    (set_attr "length" "2,4")
@@ -4304,7 +4304,7 @@ (define_insn "unaligned_storesi"
   [(set (match_operand:SI 0 "memory_operand" "=Uw,m")
 	(unspec:SI [(match_operand:SI 1 "s_register_operand" "l,r")]
 		   UNSPEC_UNALIGNED_STORE))]
-  "unaligned_access && TARGET_32BIT"
+  "unaligned_access"
   "str%?\t%1, %0\t@ unaligned"
   [(set_attr "arch" "t2,any")
    (set_attr "length" "2,4")
@@ -4316,7 +4316,7 @@ (define_insn "unaligned_storehi"
   [(set (match_operand:HI 0 "memory_operand" "=Uw,m")
 	(unspec:HI [(match_operand:HI 1 "s_register_operand" "l,r")]
 		   UNSPEC_UNALIGNED_STORE))]
-  "unaligned_access && TARGET_32BIT"
+  "unaligned_access"
   "strh%?\t%1, %0\t@ unaligned"
   [(set_attr "arch" "t2,any")
    (set_attr "length" "2,4")
diff --git a/gcc/testsuite/gcc.target/arm/armv6-unaligned-load-ice.c b/gcc/testsuite/gcc.target/arm/armv6-unaligned-load-ice.c
new file mode 100644
index 0000000..88528f1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/armv6-unaligned-load-ice.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6k" } } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-marm" } { "" } } */
+/* { dg-options "-mthumb -Os -mfloat-abi=softfp" } */
+/* { dg-add-options arm_arch_v6k } */
+
+long
+get_number (char *s, long size, int unsigned_p)
+{
+  long x;
+  unsigned char *p = (unsigned char *) s;
+  switch (size)
+    {
+    case 4:
+      x = ((long) p[3] << 24) | ((long) p[2] << 16) | (p[1] << 8) | p[0];
+      return x;
+    }
+}

Re: [PATCH][ARM] Do not expand movmisalign pattern if not in 32-bit mode

[Kyrill Tkachov]

Ping.
https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01392.html

Thanks,
Kyrill

On 11/11/15 16:10, Kyrill Tkachov wrote:
> Hi all,
>
> The attached testcase ICEs when compiled with -march=armv6k -mthumb -Os or any march
> for which -mthumb gives Thumb1:
>  error: unrecognizable insn:
>  }
>  ^
> (insn 13 12 14 5 (set (reg:SI 116 [ x ])
>         (unspec:SI [
>                 (mem:SI (reg/v/f:SI 112 [ s ]) [0 MEM[(unsigned char *)s_1(D)]+0 S4 A8])
>             ] UNSPEC_UNALIGNED_LOAD)) besttry.c:9 -1
>      (nil))
>
> The problem is that the expands a movmisalign pattern but the resulting unaligned loads don't
> match any define_insn because they are gated on unaligned_access && TARGET_32BIT.
> The unaligned_access expander is gated only on unaligned_access.
>
> This small patch fixes the issue by turning off unaligned_access if TARGET_32BIT is not true.
> We can then remove TARGET_32BIT from the unaligned load/store patterns conditions as a cleanup.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2015-11-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/arm/arm.c (arm_option_override): Require TARGET_32BIT
>     for unaligned_access.
>     * config/arm/arm.md (unaligned_loadsi): Remove redundant TARGET_32BIT
>     from matching condition.
>     (unaligned_loadhis): Likewise.
>     (unaligned_loadhiu): Likewise.
>     (unaligned_storesi): Likewise.
>     (unaligned_storehi): Likewise.
>
> 2015-11-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * gcc.target/arm/armv6-unaligned-load-ice.c: New test.

Re: [PATCH][ARM] Do not expand movmisalign pattern if not in 32-bit mode

[Ramana Radhakrishnan]

On 11/11/15 16:10, Kyrill Tkachov wrote:
> Hi all,
> 
> The attached testcase ICEs when compiled with -march=armv6k -mthumb -Os or any march
> for which -mthumb gives Thumb1:
>  error: unrecognizable insn:
>  }
>  ^
> (insn 13 12 14 5 (set (reg:SI 116 [ x ])
>         (unspec:SI [
>                 (mem:SI (reg/v/f:SI 112 [ s ]) [0 MEM[(unsigned char *)s_1(D)]+0 S4 A8])
>             ] UNSPEC_UNALIGNED_LOAD)) besttry.c:9 -1
>      (nil))
> 
> The problem is that the expands a movmisalign pattern but the resulting unaligned loads don't
> match any define_insn because they are gated on unaligned_access && TARGET_32BIT.
> The unaligned_access expander is gated only on unaligned_access.
> 
> This small patch fixes the issue by turning off unaligned_access if TARGET_32BIT is not true.
> We can then remove TARGET_32BIT from the unaligned load/store patterns conditions as a cleanup.
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> 
> Ok for trunk?
> 
> Thanks,
> Kyrill
> 
> 2015-11-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/arm/arm.c (arm_option_override): Require TARGET_32BIT
>     for unaligned_access.
>     * config/arm/arm.md (unaligned_loadsi): Remove redundant TARGET_32BIT
>     from matching condition.
>     (unaligned_loadhis): Likewise.
>     (unaligned_loadhiu): Likewise.
>     (unaligned_storesi): Likewise.
>     (unaligned_storehi): Likewise.
> 
> 2015-11-11  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * gcc.target/arm/armv6-unaligned-load-ice.c: New test.


This means we don't have unaligned access for some cores in Thumb1 for armv6. I'd rather not have the ICE instead of trying to find testing coverage on such cores now.

OK.


regards
Ramana