diff mbox

[SH4] Fix PR58314 (unsatisfied constraints)

Message ID 5231C3AB.3040401@st.com
State New
Headers show

Commit Message

Christian Bruel Sept. 12, 2013, 1:37 p.m. UTC 
The attached patch fixes an ice while building the linux kernel. Reduced
in the included testcase.

The problem is that we are generating a movhi_reg_reg insn that accepts
only registers as operands. Spilling a pseudo on the stack results in an
invalid memory load/store constraints.

The attached patch allows memory for reload.
Tested with the testsuite on sh4-linux and sh-superh-gcc.
No performance impact on a large number of benchmarks (EEMBC, CSIBe,
spec2006, ...)

Oleg, since you moved out the r,r constraints from *mohi into
movhi_reg_reg, do you agree ?

OK for trunk and 4.8 ?

Thanks

Christian

Comments

Oleg Endo Sept. 12, 2013, 10:25 p.m. UTC | #1 
On Thu, 2013-09-12 at 15:37 +0200, Christian Bruel wrote:
> The attached patch fixes an ice while building the linux kernel. Reduced
> in the included testcase.
> 
> The problem is that we are generating a movhi_reg_reg insn that accepts
> only registers as operands. Spilling a pseudo on the stack results in an
> invalid memory load/store constraints.
> 
> The attached patch allows memory for reload.
> Tested with the testsuite on sh4-linux and sh-superh-gcc.
> No performance impact on a large number of benchmarks (EEMBC, CSIBe,
> spec2006, ...)
> 
> Oleg, since you moved out the r,r constraints from *mohi into
> movhi_reg_reg, do you agree ?

Yep.  Just a few nits:

- the comment block above the "*mov<mode>_reg_reg" pattern is partially
invalidated by your fix and should be updated, too.

- although the original failure popped up with -Os, I think the test
should go into gcc/testsuite/gcc.target/sh/torture/

Cheers,
Oleg
Kaz Kojima Sept. 12, 2013, 11:01 p.m. UTC | #2 
Christian Bruel <christian.bruel@st.com> wrote:
> The attached patch fixes an ice while building the linux kernel. Reduced
> in the included testcase.
> 
> The problem is that we are generating a movhi_reg_reg insn that accepts
> only registers as operands. Spilling a pseudo on the stack results in an
> invalid memory load/store constraints.
> 
> The attached patch allows memory for reload.
> Tested with the testsuite on sh4-linux and sh-superh-gcc.
> No performance impact on a large number of benchmarks (EEMBC, CSIBe,
> spec2006, ...)
> 
> Oleg, since you moved out the r,r constraints from *mohi into
> movhi_reg_reg, do you agree ?
> 
> OK for trunk and 4.8 ?
 
OK.  Updates suggested by Oleg are also fine.  When you
incorporate them into the patch, please send it to the list
for the record.

Regards,
	kaz
diff mbox

Patch

2013-09-13  Christian Bruel  <christian.bruel@st.com>

	PR target/58314
	* config/sh/sh.md (mov<mode>_reg_reg): Allow memory for reload.

2013-09-13  Christian Bruel  <christian.bruel@st.com>

	PR target/58314
	* gcc.target/sh/pr58314.c: New test.

Index: gcc/config/sh/sh.md
===================================================================
--- gcc/config/sh/sh.md	(revision 202517)
+++ gcc/config/sh/sh.md	(working copy)
@@ -6893,11 +6893,14 @@  label:
 ;; reloading MAC subregs otherwise.  For that probably special patterns
 ;; would be required.
 (define_insn "*mov<mode>_reg_reg"
-  [(set (match_operand:QIHI 0 "arith_reg_dest" "=r")
-	(match_operand:QIHI 1 "register_operand" "r"))]
+  [(set (match_operand:QIHI 0 "arith_reg_dest" "=r,m,*z")
+	(match_operand:QIHI 1 "register_operand" "r,*z,m"))]
   "TARGET_SH1 && !t_reg_operand (operands[1], VOIDmode)"
-  "mov	%1,%0"
-  [(set_attr "type" "move")])
+  "@
+    mov		%1,%0
+    mov.<bw>	%1,%0
+    mov.<bw>	%1,%0"
+  [(set_attr "type" "move,store,load")])
 
 ;; FIXME: The non-SH2A and SH2A variants should be combined by adding
 ;; "enabled" attribute as it is done in other targets.
Index: gcc/testsuite/gcc.target/sh/pr58314.c
===================================================================
--- gcc/testsuite/gcc.target/sh/pr58314.c	(revision 0)
+++ gcc/testsuite/gcc.target/sh/pr58314.c	(working copy)
@@ -0,0 +1,102 @@ 
+/* { dg-do compile { target "sh*-*-*" } } */
+/* { dg-options "-Os" } */
+
+typedef unsigned short __u16;
+typedef unsigned int __u32;
+
+typedef signed short s16;
+
+
+static inline __attribute__((always_inline)) __attribute__((__const__)) __u16 __arch_swab16(__u16 x)
+{
+ __asm__(
+  "swap.b		%1, %0"
+  : "=r" (x)
+  : "r" (x));
+ return x;
+}
+
+void u16_add_cpu(__u16 *var)
+{
+  *var = __arch_swab16(*var);
+}
+
+typedef struct xfs_mount {
+ int m_attr_magicpct;
+} xfs_mount_t;
+
+typedef struct xfs_da_args {
+ struct xfs_mount *t_mountp;
+ int index;
+} xfs_da_args_t;
+
+typedef struct xfs_dabuf {
+ void *data;
+} xfs_dabuf_t;
+
+typedef struct xfs_attr_leaf_map {
+ __u16 base;
+ __u16 size;
+} xfs_attr_leaf_map_t;
+typedef struct xfs_attr_leaf_hdr {
+ __u16 count;
+ xfs_attr_leaf_map_t freemap[3];
+} xfs_attr_leaf_hdr_t;
+
+typedef struct xfs_attr_leaf_entry {
+  __u16 nameidx;
+} xfs_attr_leaf_entry_t;
+
+typedef struct xfs_attr_leafblock {
+ xfs_attr_leaf_hdr_t hdr;
+ xfs_attr_leaf_entry_t entries[1];
+} xfs_attr_leafblock_t;
+
+int
+xfs_attr_leaf_remove(xfs_attr_leafblock_t *leaf, xfs_da_args_t *args)
+{
+ xfs_attr_leaf_hdr_t *hdr;
+ xfs_attr_leaf_map_t *map;
+ xfs_attr_leaf_entry_t *entry;
+ int before, after, smallest, entsize;
+ int tablesize, tmp, i;
+ xfs_mount_t *mp;
+ hdr = &leaf->hdr;
+ mp = args->t_mountp;
+
+ entry = &leaf->entries[args->index];
+
+ tablesize = __arch_swab16(hdr->count);
+
+ map = &hdr->freemap[0];
+ tmp = map->size;
+ before = after = -1;
+ smallest = 3 - 1;
+ entsize = xfs_attr_leaf_entsize(leaf, args->index);
+
+ for (i = 0; i < 2; map++, i++) {
+
+  if (map->base == tablesize)
+    u16_add_cpu(&map->base);
+
+  if (__arch_swab16(map->base)  + __arch_swab16(map->size)  == __arch_swab16(entry->nameidx))
+   before = i;
+  else if (map->base == entsize)
+   after = i;
+  else if (__arch_swab16(map->size) < tmp)
+   smallest = i;
+ }
+
+ if (before >= 0)
+  {
+   map = &hdr->freemap[after];
+   map->base = entry->nameidx;
+
+  }
+
+  map = &hdr->freemap[smallest];
+
+  map->base = __arch_swab16(entry->nameidx);
+
+ return(tmp < mp->m_attr_magicpct);
+}