Patchwork [ARM] Fix gcc.dg/pr48335-5.c ICE with NEON enabled

login
register
mail settings
Submitter Julian Brown
Date June 17, 2013, 11:14 a.m.
Message ID <20130617121459.1443db75@octopus>
Download mbox | patch
Permalink /patch/251816/
State New
Headers show

Comments

Julian Brown - June 17, 2013, 11:14 a.m.
Hi,

This patch fixes an ICE where the compiler crashes (with NEON enabled)
during expansion of an instruction such as:

pr48335-5.c:17:1: error: unrecognizable insn:
 }
 ^
(insn 9 8 10 2 (set (reg:DI 116 [ s ])
        (unspec:DI [
                (mem/c:DI (plus:SI (reg/f:SI 105 virtual-stack-vars)
                        (const_int -8 [0xfffffffffffffff8])) [2 S8 A32])
            ] UNSPEC_MISALIGNED_ACCESS)) pr48335-5.c:15 -1
     (nil))
pr48335-5.c:17:1: internal compiler error: in extract_insn, at recog.c:2158

The problem is that generic code (expr.c:expand_expr_real_1, case
VIEW_CONVERT_EXPR) expects to be able to use movmisalign without
explicitly checking that the operands match -- reminiscent of normal
moves, which are generally expected to "always work". However, the
current predicates on the NEON movmisalign insn patterns reject MEMs
which refer to virtual registers such as the virtual-stack-vars above,
leading to the ICE, even though the middle end would generally be capable of
rewriting such an instruction into one which is valid.

The fix for this is to tweak the operands for the instruction patterns
in question to allow such virtual registers, pre-reload. This fixes
gcc.dg/pr48335-5.c when the testsuite is run with "-march=armv7-a
-mfpu=neon -mfloat-abi=softfp" options, with no other changes in
results for gcc, g++ & libstdc++.

OK to apply?

Thanks,

Julian

ChangeLog

    gcc/
    * config/arm/arm.c (neon_vector_mem_operand): Add strict argument.
    Permit virtual register pre-reload if !strict.
    (coproc_secondary_reload_class): Adjust for neon_vector_mem_operand
    change.
    * config/arm/arm-protos.h (neon_vector_mem_operand): Adjust
    prototype.
    * config/arm/neon.md (movmisalign<mode>): Use
    neon_perm_struct_or_reg_operand instead of
    neon_struct_or_register_operand.
    (*movmisalign<mode>_neon_load, *movmisalign<mode>_neon_store): Use
    neon_permissive_struct_operand instead of neon_struct_operand.
    * config/arm/constraints.md (Un, Um, Us): Adjust calls to
    neon_vector_mem_operand.
    * config/arm/predicates.md (neon_struct_operand): Adjust call to
    neon_vector_mem_operand.
    (neon_permissive_struct_operand): New.
    (neon_struct_or_register_operand): Rename to...
    (neon_perm_struct_or_reg_operand): This. Adjust call to
    neon_vector_mem_operand.
Ramana Radhakrishnan - June 18, 2013, 9:54 a.m.
>
> Thanks,
>
> Julian
>
> ChangeLog
>
>      gcc/
>      * config/arm/arm.c (neon_vector_mem_operand): Add strict argument.
>      Permit virtual register pre-reload if !strict.
>      (coproc_secondary_reload_class): Adjust for neon_vector_mem_operand
>      change.
>      * config/arm/arm-protos.h (neon_vector_mem_operand): Adjust
>      prototype.
>      * config/arm/neon.md (movmisalign<mode>): Use
>      neon_perm_struct_or_reg_operand instead of
>      neon_struct_or_register_operand.
>      (*movmisalign<mode>_neon_load, *movmisalign<mode>_neon_store): Use
>      neon_permissive_struct_operand instead of neon_struct_operand.
>      * config/arm/constraints.md (Un, Um, Us): Adjust calls to
>      neon_vector_mem_operand.
>      * config/arm/predicates.md (neon_struct_operand): Adjust call to
>      neon_vector_mem_operand.
>      (neon_permissive_struct_operand): New.
>      (neon_struct_or_register_operand): Rename to...
>      (neon_perm_struct_or_reg_operand): This. Adjust call to
>      neon_vector_mem_operand.
>


Ok but this also needs to go to FSF 4.8 if no RM objects and after a few 
days of soaking on trunk.

regards
Ramana

Patch

Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 200070)
+++ gcc/config/arm/arm.c	(working copy)
@@ -7863,7 +7863,7 @@  arm_rtx_costs_1 (rtx x, enum rtx_code ou
 	  && GET_CODE (SET_SRC (x)) == VEC_SELECT)
 	{
 	  *total = rtx_cost (SET_DEST (x), code, 0, speed);
-	  if (!neon_vector_mem_operand (SET_DEST (x), 2))
+	  if (!neon_vector_mem_operand (SET_DEST (x), 2, true))
 	    *total += COSTS_N_INSNS (1);
 	  return true;
 	}
@@ -7874,7 +7874,7 @@  arm_rtx_costs_1 (rtx x, enum rtx_code ou
 	{
 	  rtx mem = XEXP (XEXP (SET_SRC (x), 0), 0);
 	  *total = rtx_cost (mem, code, 0, speed);
-	  if (!neon_vector_mem_operand (mem, 2))
+	  if (!neon_vector_mem_operand (mem, 2, true))
 	    *total += COSTS_N_INSNS (1);
 	  return true;
 	}
@@ -10046,7 +10046,7 @@  arm_coproc_mem_operand (rtx op, bool wb)
     2 - Element/structure loads (vld1)
  */
 int
-neon_vector_mem_operand (rtx op, int type)
+neon_vector_mem_operand (rtx op, int type, bool strict)
 {
   rtx ind;
 
@@ -10058,7 +10058,7 @@  neon_vector_mem_operand (rtx op, int typ
 	  || reg_mentioned_p (virtual_outgoing_args_rtx, op)
 	  || reg_mentioned_p (virtual_stack_dynamic_rtx, op)
 	  || reg_mentioned_p (virtual_stack_vars_rtx, op)))
-    return FALSE;
+    return !strict;
 
   /* Constants are converted into offsets from labels.  */
   if (!MEM_P (op))
@@ -10168,7 +10168,7 @@  coproc_secondary_reload_class (enum mach
     {
       if (!TARGET_NEON_FP16)
 	return GENERAL_REGS;
-      if (s_register_operand (x, mode) || neon_vector_mem_operand (x, 2))
+      if (s_register_operand (x, mode) || neon_vector_mem_operand (x, 2, true))
 	return NO_REGS;
       return GENERAL_REGS;
     }
Index: gcc/config/arm/arm-protos.h
===================================================================
--- gcc/config/arm/arm-protos.h	(revision 200070)
+++ gcc/config/arm/arm-protos.h	(working copy)
@@ -95,7 +95,7 @@  extern enum reg_class coproc_secondary_r
 extern bool arm_tls_referenced_p (rtx);
 
 extern int arm_coproc_mem_operand (rtx, bool);
-extern int neon_vector_mem_operand (rtx, int);
+extern int neon_vector_mem_operand (rtx, int, bool);
 extern int neon_struct_mem_operand (rtx);
 extern int arm_no_early_store_addr_dep (rtx, rtx);
 extern int arm_early_store_addr_dep (rtx, rtx);
Index: gcc/config/arm/neon.md
===================================================================
--- gcc/config/arm/neon.md	(revision 200070)
+++ gcc/config/arm/neon.md	(working copy)
@@ -241,8 +241,8 @@ 
 })
 
 (define_expand "movmisalign<mode>"
-  [(set (match_operand:VDQX 0 "neon_struct_or_register_operand")
-	(unspec:VDQX [(match_operand:VDQX 1 "neon_struct_or_register_operand")]
+  [(set (match_operand:VDQX 0 "neon_perm_struct_or_reg_operand")
+	(unspec:VDQX [(match_operand:VDQX 1 "neon_perm_struct_or_reg_operand")]
 		     UNSPEC_MISALIGNED_ACCESS))]
   "TARGET_NEON && !BYTES_BIG_ENDIAN"
 {
@@ -255,7 +255,7 @@ 
 })
 
 (define_insn "*movmisalign<mode>_neon_store"
-  [(set (match_operand:VDX 0 "neon_struct_operand"	       "=Um")
+  [(set (match_operand:VDX 0 "neon_permissive_struct_operand"	"=Um")
 	(unspec:VDX [(match_operand:VDX 1 "s_register_operand" " w")]
 		    UNSPEC_MISALIGNED_ACCESS))]
   "TARGET_NEON && !BYTES_BIG_ENDIAN"
@@ -263,15 +263,16 @@ 
   [(set_attr "neon_type" "neon_vst1_1_2_regs_vst2_2_regs")])
 
 (define_insn "*movmisalign<mode>_neon_load"
-  [(set (match_operand:VDX 0 "s_register_operand"		"=w")
-	(unspec:VDX [(match_operand:VDX 1 "neon_struct_operand" " Um")]
+  [(set (match_operand:VDX 0 "s_register_operand"			"=w")
+	(unspec:VDX [(match_operand:VDX 1 "neon_permissive_struct_operand"
+									" Um")]
 		    UNSPEC_MISALIGNED_ACCESS))]
   "TARGET_NEON && !BYTES_BIG_ENDIAN"
   "vld1.<V_sz_elem>\t{%P0}, %A1"
   [(set_attr "neon_type" "neon_vld1_1_2_regs")])
 
 (define_insn "*movmisalign<mode>_neon_store"
-  [(set (match_operand:VQX 0 "neon_struct_operand"	       "=Um")
+  [(set (match_operand:VQX 0 "neon_permissive_struct_operand"  "=Um")
 	(unspec:VQX [(match_operand:VQX 1 "s_register_operand" " w")]
 		    UNSPEC_MISALIGNED_ACCESS))]
   "TARGET_NEON && !BYTES_BIG_ENDIAN"
@@ -279,8 +280,9 @@ 
   [(set_attr "neon_type" "neon_vst1_1_2_regs_vst2_2_regs")])
 
 (define_insn "*movmisalign<mode>_neon_load"
-  [(set (match_operand:VQX 0 "s_register_operand"	         "=w")
-	(unspec:VQX [(match_operand:VQX 1 "neon_struct_operand" " Um")]
+  [(set (match_operand:VQX 0 "s_register_operand"			"=w")
+	(unspec:VQX [(match_operand:VQX 1 "neon_permissive_struct_operand"
+									" Um")]
 		    UNSPEC_MISALIGNED_ACCESS))]
   "TARGET_NEON && !BYTES_BIG_ENDIAN"
   "vld1.<V_sz_elem>\t{%q0}, %A1"
Index: gcc/config/arm/constraints.md
===================================================================
--- gcc/config/arm/constraints.md	(revision 200070)
+++ gcc/config/arm/constraints.md	(working copy)
@@ -358,21 +358,21 @@ 
   In ARM/Thumb-2 state a valid address for Neon doubleword vector
   load/store instructions."
  (and (match_code "mem")
-      (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 0)")))
+      (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 0, true)")))
 
 (define_memory_constraint "Um"
  "@internal
   In ARM/Thumb-2 state a valid address for Neon element and structure
   load/store instructions."
  (and (match_code "mem")
-      (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 2)")))
+      (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 2, true)")))
 
 (define_memory_constraint "Us"
  "@internal
   In ARM/Thumb-2 state a valid address for non-offset loads/stores of
   quad-word values in four ARM registers."
  (and (match_code "mem")
-      (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 1)")))
+      (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 1, true)")))
 
 (define_memory_constraint "Uq"
  "@internal
Index: gcc/config/arm/predicates.md
===================================================================
--- gcc/config/arm/predicates.md	(revision 200070)
+++ gcc/config/arm/predicates.md	(working copy)
@@ -634,10 +634,14 @@ 
 
 (define_predicate "neon_struct_operand"
   (and (match_code "mem")
-       (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 2)")))
+       (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 2, true)")))
 
-(define_predicate "neon_struct_or_register_operand"
-  (ior (match_operand 0 "neon_struct_operand")
+(define_predicate "neon_permissive_struct_operand"
+  (and (match_code "mem")
+       (match_test "TARGET_32BIT && neon_vector_mem_operand (op, 2, false)")))
+
+(define_predicate "neon_perm_struct_or_reg_operand"
+  (ior (match_operand 0 "neon_permissive_struct_operand")
        (match_operand 0 "s_register_operand")))
 
 (define_special_predicate "add_operator"