Patchwork [ARM] Fix PR46883, ICE at reload

login
register
mail settings
Submitter Chung-Lin Tang
Date Dec. 14, 2010, 2:05 p.m.
Message ID <4D0779A3.6070200@codesourcery.com>
Download mbox | patch
Permalink /patch/75491/
State New
Headers show

Comments

Chung-Lin Tang - Dec. 14, 2010, 2:05 p.m.
Hi, this patch tries to fix PR46883, where reload ICEs at an 
unrecognized insn.

The failing insn is produced during split1. The "register_operand" 
predicate allows (subreg (mem)) before reload, which allows the 
zero_extend_qisi2 register splitter to hook on and create (subreg:QI 
(mem:HI (post_inc ...))), which causes reload to ICE (in two different 
places, depending on -O1 or -O2).

While reload may need further work later, this patch changes the 
predicates above to use the ARM backend's "s_register_operand", which 
matches only regs/subregs of regs. This resolves the ICE, and seems to 
more closely match the original intention of the splitters.

The PR testcase triggers the ICE due to the QI/SI splitter, but I have 
fixed the HI/SI splitter too, as it seems to be analogous.

Tested without regressions. Ok to commit?

Thanks,
Chung-Lin

2010-12-14  Chung-Lin Tang  <cltang@codesourcery.com>

	PR target/46883
	* config/arm/arm.md
	(zero_extendhisi2 for register input splitter): Change
	"register_operand" to "s_register_operand".
	(zero_extendqisi2 for register input splitter): Same.
Richard Earnshaw - Dec. 14, 2010, 5:39 p.m.
On Tue, 2010-12-14 at 22:05 +0800, Chung-Lin Tang wrote:
> Hi, this patch tries to fix PR46883, where reload ICEs at an 
> unrecognized insn.
> 
> The failing insn is produced during split1. The "register_operand" 
> predicate allows (subreg (mem)) before reload, which allows the 
> zero_extend_qisi2 register splitter to hook on and create (subreg:QI 
> (mem:HI (post_inc ...))), which causes reload to ICE (in two different 
> places, depending on -O1 or -O2).
> 
> While reload may need further work later, this patch changes the 
> predicates above to use the ARM backend's "s_register_operand", which 
> matches only regs/subregs of regs. This resolves the ICE, and seems to 
> more closely match the original intention of the splitters.
> 
> The PR testcase triggers the ICE due to the QI/SI splitter, but I have 
> fixed the HI/SI splitter too, as it seems to be analogous.
> 
> Tested without regressions. Ok to commit?
> 
> Thanks,
> Chung-Lin
> 
> 2010-12-14  Chung-Lin Tang  <cltang@codesourcery.com>
> 
> 	PR target/46883
> 	* config/arm/arm.md
> 	(zero_extendhisi2 for register input splitter): Change
> 	"register_operand" to "s_register_operand".
> 	(zero_extendqisi2 for register input splitter): Same.

OK.

R.

Patch

Index: config/arm/arm.md
===================================================================
--- config/arm/arm.md	(revision 167797)
+++ config/arm/arm.md	(working copy)
@@ -4137,8 +4137,8 @@ 
 })
 
 (define_split
-  [(set (match_operand:SI 0 "register_operand" "")
-	(zero_extend:SI (match_operand:HI 1 "register_operand" "")))]
+  [(set (match_operand:SI 0 "s_register_operand" "")
+	(zero_extend:SI (match_operand:HI 1 "s_register_operand" "")))]
   "!TARGET_THUMB2 && !arm_arch6"
   [(set (match_dup 0) (ashift:SI (match_dup 2) (const_int 16)))
    (set (match_dup 0) (lshiftrt:SI (match_dup 0) (const_int 16)))]
@@ -4244,8 +4244,8 @@ 
 })
 
 (define_split
-  [(set (match_operand:SI 0 "register_operand" "")
-	(zero_extend:SI (match_operand:QI 1 "register_operand" "")))]
+  [(set (match_operand:SI 0 "s_register_operand" "")
+	(zero_extend:SI (match_operand:QI 1 "s_register_operand" "")))]
   "!arm_arch6"
   [(set (match_dup 0) (ashift:SI (match_dup 2) (const_int 24)))
    (set (match_dup 0) (lshiftrt:SI (match_dup 0) (const_int 24)))]
Index: testsuite/gcc.target/arm/pr46883.c
===================================================================
--- testsuite/gcc.target/arm/pr46883.c	(revision 0)
+++ testsuite/gcc.target/arm/pr46883.c	(revision 0)
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -march=armv5te" } */
+
+void bar (unsigned char *q, unsigned short *data16s, int len)
+{
+  int i;
+
+  for (i = 0; i < len; i++)
+    {
+      q[2 * i] =
+        (((data16s[i] & 0xFF) << 8) | ((data16s[i] >> 8) & 0xFF)) & 0xFF;
+      q[2 * i + 1] =
+        ((unsigned short)
+         (((data16s[i] & 0xFF) << 8) | ((data16s[i] >> 8) & 0xFF))) >> 8;
+    }
+}