[PATCH 0/5] MIPS: LLVMLinux: Patches to enable compilation of a working kernel for MIPS using Clang/LLVM

[PATCH 0/5] MIPS: LLVMLinux: Patches to enable compilation of a working kernel for MIPS using Clang/LLVM

[Daniel Sanders]

When combined with 'MIPS: Changed current_thread_info() to an equivalent ...'
(http://www.linux-mips.org/archives/linux-mips/2015-01/msg00070.html), this
patch series makes it possible to compile a working kernel for MIPS using Clang.

The patches aren't inter-dependent so I'm happy to split this up into individual
submissions if that's preferred.

Daniel Sanders (1):
  LLVMLinux: Correct size_index table before replacing the bootstrap
    kmem_cache_node.

Toma Tabacu (4):
  MIPS: LLVMLinux: Fix a 'cast to type not present in union' error.
  MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.
  MIPS: LLVMLinux: Silence variable self-assignment warnings.
  MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

 arch/mips/include/asm/asmmacro.h |  8 ++++----
 arch/mips/include/asm/checksum.h |  2 +-
 arch/mips/kernel/branch.c        |  6 ++++--
 arch/mips/math-emu/dp_add.c      |  5 -----
 arch/mips/math-emu/dp_sub.c      |  5 -----
 arch/mips/math-emu/sp_add.c      |  5 -----
 arch/mips/math-emu/sp_sub.c      |  5 -----
 mm/slab.c                        |  1 +
 mm/slab.h                        |  1 +
 mm/slab_common.c                 | 36 +++++++++++++++++++++---------------
 mm/slub.c                        |  1 +
 11 files changed, 33 insertions(+), 42 deletions(-)

Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: "Steven J. Hill" <Steven.Hill@imgtec.com>
Cc: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: David Daney <david.daney@cavium.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Jim Quinlan <jim2101024@gmail.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: Manuel Lauss <manuel.lauss@gmail.com>
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-mm@kvack.org

-- 
2.1.4

[PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Daniel Sanders]

This patch moves the initialization of the size_index table slightly
earlier so that the first few kmem_cache_node's can be safely allocated
when KMALLOC_MIN_SIZE is large.

There are currently two ways to generate indices into kmalloc_caches
(via kmalloc_index() and via the size_index table in slab_common.c)
and on some arches (possibly only MIPS) they potentially disagree with
each other until create_kmalloc_caches() has been called. It seems
that the intention is that the size_index table is a fast equivalent
to kmalloc_index() and that create_kmalloc_caches() patches the table
to return the correct value for the cases where kmalloc_index()'s
if-statements apply.

The failing sequence was:
* kmalloc_caches contains NULL elements
* kmem_cache_init initialises the element that 'struct
  kmem_cache_node' will be allocated to. For 32-bit Mips, this is a
  56-byte struct and kmalloc_index returns KMALLOC_SHIFT_LOW (7).
* init_list is called which calls kmalloc_node to allocate a 'struct
  kmem_cache_node'.
* kmalloc_slab selects the kmem_caches element using
  size_index[size_index_elem(size)]. For MIPS, size is 56, and the
  expression returns 6.
* This element of kmalloc_caches is NULL and allocation fails.
* If it had not already failed, it would have called
  create_kmalloc_caches() at this point which would have changed
  size_index[size_index_elem(size)] to 7.

Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
 mm/slab.c        |  1 +
 mm/slab.h        |  1 +
 mm/slab_common.c | 36 +++++++++++++++++++++---------------
 mm/slub.c        |  1 +
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 65b5dcb..6c93f28 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1440,6 +1440,7 @@ void __init kmem_cache_init(void)
 	kmalloc_caches[INDEX_NODE] = create_kmalloc_cache("kmalloc-node",
 				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS);
 	slab_state = PARTIAL_NODE;
+	correct_kmalloc_cache_index_table();
 
 	slab_early_init = 0;
 
diff --git a/mm/slab.h b/mm/slab.h
index 1cf40054..036c08d 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -71,6 +71,7 @@ unsigned long calculate_alignment(unsigned long flags,
 
 #ifndef CONFIG_SLOB
 /* Kmalloc array related functions */
+void correct_kmalloc_cache_index_table(void);
 void create_kmalloc_caches(unsigned long);
 
 /* Find the kmalloc slab corresponding for a certain size */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index e03dd6f..d2f7379 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -675,25 +675,20 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
 }
 
 /*
- * Create the kmalloc array. Some of the regular kmalloc arrays
- * may already have been created because they were needed to
- * enable allocations for slab creation.
+ * Patch up the size_index table if we have strange large alignment
+ * requirements for the kmalloc array. This is only the case for
+ * MIPS it seems. The standard arches will not generate any code here.
+ *
+ * Largest permitted alignment is 256 bytes due to the way we
+ * handle the index determination for the smaller caches.
+ *
+ * Make sure that nothing crazy happens if someone starts tinkering
+ * around with ARCH_KMALLOC_MINALIGN
  */
-void __init create_kmalloc_caches(unsigned long flags)
+void __init correct_kmalloc_cache_index_table(void)
 {
 	int i;
 
-	/*
-	 * Patch up the size_index table if we have strange large alignment
-	 * requirements for the kmalloc array. This is only the case for
-	 * MIPS it seems. The standard arches will not generate any code here.
-	 *
-	 * Largest permitted alignment is 256 bytes due to the way we
-	 * handle the index determination for the smaller caches.
-	 *
-	 * Make sure that nothing crazy happens if someone starts tinkering
-	 * around with ARCH_KMALLOC_MINALIGN
-	 */
 	BUILD_BUG_ON(KMALLOC_MIN_SIZE > 256 ||
 		(KMALLOC_MIN_SIZE & (KMALLOC_MIN_SIZE - 1)));
 
@@ -724,6 +719,17 @@ void __init create_kmalloc_caches(unsigned long flags)
 		for (i = 128 + 8; i <= 192; i += 8)
 			size_index[size_index_elem(i)] = 8;
 	}
+}
+
+/*
+ * Create the kmalloc array. Some of the regular kmalloc arrays
+ * may already have been created because they were needed to
+ * enable allocations for slab creation.
+ */
+void __init create_kmalloc_caches(unsigned long flags)
+{
+	int i;
+
 	for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
 		if (!kmalloc_caches[i]) {
 			kmalloc_caches[i] = create_kmalloc_cache(NULL,
diff --git a/mm/slub.c b/mm/slub.c
index fe376fe..2217761 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3604,6 +3604,7 @@ void __init kmem_cache_init(void)
 	kmem_cache_node = bootstrap(&boot_kmem_cache_node);
 
 	/* Now we can use the kmem_cache to allocate kmalloc slabs */
+	correct_kmalloc_cache_index_table();
 	create_kmalloc_caches(0);
 
 #ifdef CONFIG_SMP
-- 
2.1.4

Re: [PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Christoph Lameter]

On Tue, 3 Feb 2015, Daniel Sanders wrote:

> +++ b/mm/slab.c
> @@ -1440,6 +1440,7 @@ void __init kmem_cache_init(void)
>  	kmalloc_caches[INDEX_NODE] = create_kmalloc_cache("kmalloc-node",
>  				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS);
>  	slab_state = PARTIAL_NODE;
> +	correct_kmalloc_cache_index_table();

Lets call this

	setup_kmalloc_cache_index_table

Please?

RE: [PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Daniel Sanders]

> -----Original Message-----
> From: Christoph Lameter [mailto:cl@linux.com]
> Sent: 03 February 2015 15:15
> To: Daniel Sanders
> Cc: Pekka Enberg; David Rientjes; Joonsoo Kim; Andrew Morton; linux-
> mm@kvack.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/5] LLVMLinux: Correct size_index table before
> replacing the bootstrap kmem_cache_node.
> 
> On Tue, 3 Feb 2015, Daniel Sanders wrote:
> 
> > +++ b/mm/slab.c
> > @@ -1440,6 +1440,7 @@ void __init kmem_cache_init(void)
> >  	kmalloc_caches[INDEX_NODE] = create_kmalloc_cache("kmalloc-
> node",
> >  				kmalloc_size(INDEX_NODE),
> ARCH_KMALLOC_FLAGS);
> >  	slab_state = PARTIAL_NODE;
> > +	correct_kmalloc_cache_index_table();
> 
> Lets call this
> 
> 	setup_kmalloc_cache_index_table
> 
> Please?

Sure, I've made the change in my repo. I'll wait a bit before re-sending the patch in case others have feedback too.

[PATCH v2 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Daniel Sanders]

This patch moves the initialization of the size_index table slightly
earlier so that the first few kmem_cache_node's can be safely allocated
when KMALLOC_MIN_SIZE is large.

There are currently two ways to generate indices into kmalloc_caches
(via kmalloc_index() and via the size_index table in slab_common.c)
and on some arches (possibly only MIPS) they potentially disagree with
each other until create_kmalloc_caches() has been called. It seems
that the intention is that the size_index table is a fast equivalent
to kmalloc_index() and that create_kmalloc_caches() patches the table
to return the correct value for the cases where kmalloc_index()'s
if-statements apply.

The failing sequence was:
* kmalloc_caches contains NULL elements
* kmem_cache_init initialises the element that 'struct
  kmem_cache_node' will be allocated to. For 32-bit Mips, this is a
  56-byte struct and kmalloc_index returns KMALLOC_SHIFT_LOW (7).
* init_list is called which calls kmalloc_node to allocate a 'struct
  kmem_cache_node'.
* kmalloc_slab selects the kmem_caches element using
  size_index[size_index_elem(size)]. For MIPS, size is 56, and the
  expression returns 6.
* This element of kmalloc_caches is NULL and allocation fails.
* If it had not already failed, it would have called
  create_kmalloc_caches() at this point which would have changed
  size_index[size_index_elem(size)] to 7.

Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---

Renamed correct_kmalloc_cache_index_table() to setup_kmalloc_cache_index_table()
as requested.

 mm/slab.c        |  1 +
 mm/slab.h        |  1 +
 mm/slab_common.c | 36 +++++++++++++++++++++---------------
 mm/slub.c        |  1 +
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 65b5dcb..d476181 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1440,6 +1440,7 @@ void __init kmem_cache_init(void)
 	kmalloc_caches[INDEX_NODE] = create_kmalloc_cache("kmalloc-node",
 				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS);
 	slab_state = PARTIAL_NODE;
+	setup_kmalloc_cache_index_table();
 
 	slab_early_init = 0;
 
diff --git a/mm/slab.h b/mm/slab.h
index 1cf40054..6121dcc 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -71,6 +71,7 @@ unsigned long calculate_alignment(unsigned long flags,
 
 #ifndef CONFIG_SLOB
 /* Kmalloc array related functions */
+void setup_kmalloc_cache_index_table(void);
 void create_kmalloc_caches(unsigned long);
 
 /* Find the kmalloc slab corresponding for a certain size */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index e03dd6f..fb45b5a 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -675,25 +675,20 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
 }
 
 /*
- * Create the kmalloc array. Some of the regular kmalloc arrays
- * may already have been created because they were needed to
- * enable allocations for slab creation.
+ * Patch up the size_index table if we have strange large alignment
+ * requirements for the kmalloc array. This is only the case for
+ * MIPS it seems. The standard arches will not generate any code here.
+ *
+ * Largest permitted alignment is 256 bytes due to the way we
+ * handle the index determination for the smaller caches.
+ *
+ * Make sure that nothing crazy happens if someone starts tinkering
+ * around with ARCH_KMALLOC_MINALIGN
  */
-void __init create_kmalloc_caches(unsigned long flags)
+void __init setup_kmalloc_cache_index_table(void)
 {
 	int i;
 
-	/*
-	 * Patch up the size_index table if we have strange large alignment
-	 * requirements for the kmalloc array. This is only the case for
-	 * MIPS it seems. The standard arches will not generate any code here.
-	 *
-	 * Largest permitted alignment is 256 bytes due to the way we
-	 * handle the index determination for the smaller caches.
-	 *
-	 * Make sure that nothing crazy happens if someone starts tinkering
-	 * around with ARCH_KMALLOC_MINALIGN
-	 */
 	BUILD_BUG_ON(KMALLOC_MIN_SIZE > 256 ||
 		(KMALLOC_MIN_SIZE & (KMALLOC_MIN_SIZE - 1)));
 
@@ -724,6 +719,17 @@ void __init create_kmalloc_caches(unsigned long flags)
 		for (i = 128 + 8; i <= 192; i += 8)
 			size_index[size_index_elem(i)] = 8;
 	}
+}
+
+/*
+ * Create the kmalloc array. Some of the regular kmalloc arrays
+ * may already have been created because they were needed to
+ * enable allocations for slab creation.
+ */
+void __init create_kmalloc_caches(unsigned long flags)
+{
+	int i;
+
 	for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
 		if (!kmalloc_caches[i]) {
 			kmalloc_caches[i] = create_kmalloc_cache(NULL,
diff --git a/mm/slub.c b/mm/slub.c
index fe376fe..11abd57 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3604,6 +3604,7 @@ void __init kmem_cache_init(void)
 	kmem_cache_node = bootstrap(&boot_kmem_cache_node);
 
 	/* Now we can use the kmem_cache to allocate kmalloc slabs */
+	setup_kmalloc_cache_index_table();
 	create_kmalloc_caches(0);
 
 #ifdef CONFIG_SMP
-- 
2.1.4

[PATCH v3 1/5] slab: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Daniel Sanders]

This patch moves the initialization of the size_index table slightly
earlier so that the first few kmem_cache_node's can be safely allocated
when KMALLOC_MIN_SIZE is large.

There are currently two ways to generate indices into kmalloc_caches
(via kmalloc_index() and via the size_index table in slab_common.c)
and on some arches (possibly only MIPS) they potentially disagree with
each other until create_kmalloc_caches() has been called. It seems
that the intention is that the size_index table is a fast equivalent
to kmalloc_index() and that create_kmalloc_caches() patches the table
to return the correct value for the cases where kmalloc_index()'s
if-statements apply.

The failing sequence was:
* kmalloc_caches contains NULL elements
* kmem_cache_init initialises the element that 'struct
  kmem_cache_node' will be allocated to. For 32-bit Mips, this is a
  56-byte struct and kmalloc_index returns KMALLOC_SHIFT_LOW (7).
* init_list is called which calls kmalloc_node to allocate a 'struct
  kmem_cache_node'.
* kmalloc_slab selects the kmem_caches element using
  size_index[size_index_elem(size)]. For MIPS, size is 56, and the
  expression returns 6.
* This element of kmalloc_caches is NULL and allocation fails.
* If it had not already failed, it would have called
  create_kmalloc_caches() at this point which would have changed
  size_index[size_index_elem(size)] to 7.

Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---

Renamed correct_kmalloc_cache_index_table() to setup_kmalloc_cache_index_table()
as requested.

I don't believe the bug to be LLVM specific but GCC doesn't normally encounter
the problem. I haven't been able to identify exactly what GCC is doing better
(probably inlining) but it seems that GCC is managing to optimize to the point
that it eliminates the problematic allocations. This theory is supported by the
fact that GCC can be made to fail in the same way by changing inline, __inline,
__inline__, and __always_inline in include/linux/compiler-gcc.h such that they
don't actually inline things.

 mm/slab.c        |  1 +
 mm/slab.h        |  1 +
 mm/slab_common.c | 36 +++++++++++++++++++++---------------
 mm/slub.c        |  1 +
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 65b5dcb..d476181 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1440,6 +1440,7 @@ void __init kmem_cache_init(void)
 	kmalloc_caches[INDEX_NODE] = create_kmalloc_cache("kmalloc-node",
 				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS);
 	slab_state = PARTIAL_NODE;
+	setup_kmalloc_cache_index_table();
 
 	slab_early_init = 0;
 
diff --git a/mm/slab.h b/mm/slab.h
index 1cf40054..6121dcc 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -71,6 +71,7 @@ unsigned long calculate_alignment(unsigned long flags,
 
 #ifndef CONFIG_SLOB
 /* Kmalloc array related functions */
+void setup_kmalloc_cache_index_table(void);
 void create_kmalloc_caches(unsigned long);
 
 /* Find the kmalloc slab corresponding for a certain size */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index e03dd6f..fb45b5a 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -675,25 +675,20 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
 }
 
 /*
- * Create the kmalloc array. Some of the regular kmalloc arrays
- * may already have been created because they were needed to
- * enable allocations for slab creation.
+ * Patch up the size_index table if we have strange large alignment
+ * requirements for the kmalloc array. This is only the case for
+ * MIPS it seems. The standard arches will not generate any code here.
+ *
+ * Largest permitted alignment is 256 bytes due to the way we
+ * handle the index determination for the smaller caches.
+ *
+ * Make sure that nothing crazy happens if someone starts tinkering
+ * around with ARCH_KMALLOC_MINALIGN
  */
-void __init create_kmalloc_caches(unsigned long flags)
+void __init setup_kmalloc_cache_index_table(void)
 {
 	int i;
 
-	/*
-	 * Patch up the size_index table if we have strange large alignment
-	 * requirements for the kmalloc array. This is only the case for
-	 * MIPS it seems. The standard arches will not generate any code here.
-	 *
-	 * Largest permitted alignment is 256 bytes due to the way we
-	 * handle the index determination for the smaller caches.
-	 *
-	 * Make sure that nothing crazy happens if someone starts tinkering
-	 * around with ARCH_KMALLOC_MINALIGN
-	 */
 	BUILD_BUG_ON(KMALLOC_MIN_SIZE > 256 ||
 		(KMALLOC_MIN_SIZE & (KMALLOC_MIN_SIZE - 1)));
 
@@ -724,6 +719,17 @@ void __init create_kmalloc_caches(unsigned long flags)
 		for (i = 128 + 8; i <= 192; i += 8)
 			size_index[size_index_elem(i)] = 8;
 	}
+}
+
+/*
+ * Create the kmalloc array. Some of the regular kmalloc arrays
+ * may already have been created because they were needed to
+ * enable allocations for slab creation.
+ */
+void __init create_kmalloc_caches(unsigned long flags)
+{
+	int i;
+
 	for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
 		if (!kmalloc_caches[i]) {
 			kmalloc_caches[i] = create_kmalloc_cache(NULL,
diff --git a/mm/slub.c b/mm/slub.c
index fe376fe..11abd57 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3604,6 +3604,7 @@ void __init kmem_cache_init(void)
 	kmem_cache_node = bootstrap(&boot_kmem_cache_node);
 
 	/* Now we can use the kmem_cache to allocate kmalloc slabs */
+	setup_kmalloc_cache_index_table();
 	create_kmalloc_caches(0);
 
 #ifdef CONFIG_SMP
-- 
2.1.4

Re: [PATCH v3 1/5] slab: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Pekka Enberg]

On 02/04/2015 11:06 PM, Daniel Sanders wrote:
> This patch moves the initialization of the size_index table slightly
> earlier so that the first few kmem_cache_node's can be safely allocated
> when KMALLOC_MIN_SIZE is large.
>
> There are currently two ways to generate indices into kmalloc_caches
> (via kmalloc_index() and via the size_index table in slab_common.c)
> and on some arches (possibly only MIPS) they potentially disagree with
> each other until create_kmalloc_caches() has been called. It seems
> that the intention is that the size_index table is a fast equivalent
> to kmalloc_index() and that create_kmalloc_caches() patches the table
> to return the correct value for the cases where kmalloc_index()'s
> if-statements apply.
>
> The failing sequence was:
> * kmalloc_caches contains NULL elements
> * kmem_cache_init initialises the element that 'struct
>    kmem_cache_node' will be allocated to. For 32-bit Mips, this is a
>    56-byte struct and kmalloc_index returns KMALLOC_SHIFT_LOW (7).
> * init_list is called which calls kmalloc_node to allocate a 'struct
>    kmem_cache_node'.
> * kmalloc_slab selects the kmem_caches element using
>    size_index[size_index_elem(size)]. For MIPS, size is 56, and the
>    expression returns 6.
> * This element of kmalloc_caches is NULL and allocation fails.
> * If it had not already failed, it would have called
>    create_kmalloc_caches() at this point which would have changed
>    size_index[size_index_elem(size)] to 7.
>
> Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org

Acked-by: Pekka Enberg <penberg@kernel.org>

Re: [PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Pekka Enberg]

On 2/3/15 3:37 PM, Daniel Sanders wrote:
> This patch moves the initialization of the size_index table slightly
> earlier so that the first few kmem_cache_node's can be safely allocated
> when KMALLOC_MIN_SIZE is large.

The patch looks OK to me but how is this related to LLVM?

- Pekka

RE: [PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Daniel Sanders]

> -----Original Message-----
> From: Pekka Enberg [mailto:penberg@iki.fi]
> Sent: 04 February 2015 19:33
> To: Daniel Sanders
> Cc: Christoph Lameter; Pekka Enberg; David Rientjes; Joonsoo Kim; Andrew
> Morton; linux-mm@kvack.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/5] LLVMLinux: Correct size_index table before
> replacing the bootstrap kmem_cache_node.
> 
> On 2/3/15 3:37 PM, Daniel Sanders wrote:
> > This patch moves the initialization of the size_index table slightly
> > earlier so that the first few kmem_cache_node's can be safely allocated
> > when KMALLOC_MIN_SIZE is large.
> 
> The patch looks OK to me but how is this related to LLVM?
>
> - Pekka

I don't believe the bug to be LLVM specific but GCC doesn't normally encounter the problem. I haven't been able to identify exactly what GCC is doing better (probably inlining) but it seems that GCC is managing to optimize  to the point that it eliminates the problematic allocations. This theory is supported by the fact that GCC can be made to fail in the same way by changing inline, __inline, __inline__, and __always_inline in include/linux/compiler-gcc.h such that they don't actually inline things.

Re: [PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Pekka Enberg]

On Wed, Feb 4, 2015 at 10:38 PM, Daniel Sanders
<Daniel.Sanders@imgtec.com> wrote:
> I don't believe the bug to be LLVM specific but GCC doesn't normally encounter the problem. I haven't been able to identify exactly what GCC is doing better (probably inlining) but it seems that GCC is managing to optimize  to the point that it eliminates the problematic allocations. This theory is supported by the fact that GCC can be made to fail in the same way by changing inline, __inline, __inline__, and __always_inline in include/linux/compiler-gcc.h such that they don't actually inline things.

OK, makes sense. Please include that explanation in the changelog and
drop use proper "slab" prefix instead of the confusing "LLVMLinux"
prefix in the subject line.

- Pekka

RE: [PATCH 1/5] LLVMLinux: Correct size_index table before replacing the bootstrap kmem_cache_node.

[Daniel Sanders]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1410 bytes --]

> -----Original Message-----
> From: penberg@gmail.com [mailto:penberg@gmail.com] On Behalf Of
> Pekka Enberg
> Sent: 04 February 2015 20:42
> To: Daniel Sanders
> Cc: Christoph Lameter; David Rientjes; Joonsoo Kim; Andrew Morton; linux-
> mm@kvack.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/5] LLVMLinux: Correct size_index table before
> replacing the bootstrap kmem_cache_node.
> 
> On Wed, Feb 4, 2015 at 10:38 PM, Daniel Sanders
> <Daniel.Sanders@imgtec.com> wrote:
> > I don't believe the bug to be LLVM specific but GCC doesn't normally
> encounter the problem. I haven't been able to identify exactly what GCC is
> doing better (probably inlining) but it seems that GCC is managing to
> optimize  to the point that it eliminates the problematic allocations. This
> theory is supported by the fact that GCC can be made to fail in the same way
> by changing inline, __inline, __inline__, and __always_inline in
> include/linux/compiler-gcc.h such that they don't actually inline things.
> 
> OK, makes sense. Please include that explanation in the changelog and
> drop use proper "slab" prefix instead of the confusing "LLVMLinux"
> prefix in the subject line.
> 
> - Pekka

Sure. I've just updated the patch with those changes.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH 2/5] MIPS: LLVMLinux: Fix a 'cast to type not present in union' error.

[Daniel Sanders]

From: Toma Tabacu <toma.tabacu@imgtec.com>

Remove a cast to the 'mips16e_instruction' union inside an if
condition and instead do an assignment to a local
'union mips16e_instruction' variable's 'full' member before the if
statement and use this variable in the if condition.

This is the error message reported by clang:
arch/mips/kernel/branch.c:38:8: error: cast to union type from type 'unsigned short' not present in union
                if (((union mips16e_instruction)inst).ri.opcode
                     ^                          ~~~~

The changed code can be compiled successfully by both gcc and clang.

Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
Cc: David Daney <david.daney@cavium.com>
Cc: Manuel Lauss <manuel.lauss@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/kernel/branch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kernel/branch.c b/arch/mips/kernel/branch.c
index 4d7d99d..7110963 100644
--- a/arch/mips/kernel/branch.c
+++ b/arch/mips/kernel/branch.c
@@ -35,8 +35,10 @@ int __isa_exception_epc(struct pt_regs *regs)
 		return epc;
 	}
 	if (cpu_has_mips16) {
-		if (((union mips16e_instruction)inst).ri.opcode
-				== MIPS16e_jal_op)
+		union mips16e_instruction inst_mips16e;
+
+		inst_mips16e.full = inst;
+		if (inst_mips16e.ri.opcode == MIPS16e_jal_op)
 			epc += 4;
 		else
 			epc += 2;
-- 
2.1.4

[PATCH 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Daniel Sanders]

From: Toma Tabacu <toma.tabacu@imgtec.com>

Change the type of csum_ipv6_magic's 'proto' argument from unsigned
short to __u32.

This fixes a type mismatch between the 'htonl(proto)' inline asm
input, which is __u32, and the 'proto' output, which is unsigned
short.

This is the error message reported by clang:
arch/mips/include/asm/checksum.h:285:27: error: unsupported inline asm: input with type '__be32' (aka 'unsigned int') matching output with type 'unsigned short'
          "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
                                 ^~~~~~~~~~~~

The changed code can be compiled successfully by both gcc and clang.

Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/include/asm/checksum.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
index 3418c51..683b9e7 100644
--- a/arch/mips/include/asm/checksum.h
+++ b/arch/mips/include/asm/checksum.h
@@ -225,7 +225,7 @@ static inline __sum16 ip_compute_csum(const void *buff, int len)
 #define _HAVE_ARCH_IPV6_CSUM
 static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 					  const struct in6_addr *daddr,
-					  __u32 len, unsigned short proto,
+					  __u32 len, __u32 proto,
 					  __wsum sum)
 {
 	__asm__(
-- 
2.1.4

Re: [PATCH 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Maciej W. Rozycki]

On Tue, 3 Feb 2015, Daniel Sanders wrote:

> From: Toma Tabacu <toma.tabacu@imgtec.com>
> 
> Change the type of csum_ipv6_magic's 'proto' argument from unsigned
> short to __u32.
> 
> This fixes a type mismatch between the 'htonl(proto)' inline asm
> input, which is __u32, and the 'proto' output, which is unsigned
> short.
> 
> This is the error message reported by clang:
> arch/mips/include/asm/checksum.h:285:27: error: unsupported inline asm: input with type '__be32' (aka 'unsigned int') matching output with type 'unsigned short'
>           "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
>                                  ^~~~~~~~~~~~
> 
> The changed code can be compiled successfully by both gcc and clang.

 This definitely looks like a bug in clang to me.  What this construct 
means is both input #5 and output #1 live in the same register, and that 
an `__u32' value is taken on input (from the result of the `htonl(proto)' 
calculation) and an `unsigned short' value produced in the same register 
on output, that'll be the value of the `proto' variable from there on.  A 
perfectly valid arrangement.  This would be the right arrangement to use 
with the MIPS16 SEH instruction for example.  Has this bug been reported 
to clang maintainers?

 And I'd prefer to leave the declaration of `proto' alone as IPv6 network 
protocol numbers are 16-bit quantities.

 That said this code is indeed weird if not wrong, which is probably why 
this arrangement resulted, in an attempt to prevent GCC from messing up 
the registers used.

 First and foremost both outputs, and especially #1, lack an earlyclobber.  
This I imagine may have prompted GCC to overwrite one of the inputs, which 
in turn is why whoever poked at this code decided to alias input #5 to 
output #1.  But as you can see in the asm there's no real aliasing between 
input #5 and output #1.  Input #5 is consumed early on (and even referred 
to with `%5' rather than `%1', which would be the norm in the case of 
actual aliasing), and the containing register reused for something else.  
So the two operands can be separated.  This is unlike input #4 vs output 
#0, that is both read and written right away (and just as one'd expect 
there's no reference to `%4' anywhere).

 Output #0 can do without an earlyclobber as it is aliased to input #4 and 
therefore cannot be assigned by GCC to another input.  But it won't hurt 
to have one too and it will set a good practice and serve a documentation 
purpose.

 I suggest a fix like this then:

static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
					  const struct in6_addr *daddr,
					  __u32 len, unsigned short proto,
					  __wsum sum)
{
	__wsum tmp;

	__asm__(
[...]
	: "=&r" (sum), "=&r" (tmp)
	: "r" (saddr), "r" (daddr),
	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));

        return csum_fold(sum);
}

Try and see if it works for you.

 I wonder why this is an asm in the first place though.  There's no rocket 
science here that GCC couldn't handle.  I guess it must have been very bad 
at optimising a C equivalent then.

  Maciej

RE: [PATCH 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Daniel Sanders]

Apologies for the slow response. I've had an excessive amount of meetings in the last couple days.

> -----Original Message-----
> From: Maciej W. Rozycki [mailto:macro@linux-mips.org]
> Sent: 04 February 2015 12:58
> To: Daniel Sanders
> Cc: Toma Tabacu; Ralf Baechle; Markos Chandras; Leonid Yegoshin; linux-
> mips@linux-mips.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output
> type mismatch' error.
> 
> On Tue, 3 Feb 2015, Daniel Sanders wrote:
> 
> > From: Toma Tabacu <toma.tabacu@imgtec.com>
> >
> > Change the type of csum_ipv6_magic's 'proto' argument from unsigned
> > short to __u32.
> >
> > This fixes a type mismatch between the 'htonl(proto)' inline asm
> > input, which is __u32, and the 'proto' output, which is unsigned
> > short.
> >
> > This is the error message reported by clang:
> > arch/mips/include/asm/checksum.h:285:27: error: unsupported inline asm:
> input with type '__be32' (aka 'unsigned int') matching output with type
> 'unsigned short'
> >           "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
> >                                  ^~~~~~~~~~~~
> >
> > The changed code can be compiled successfully by both gcc and clang.
> 
>  This definitely looks like a bug in clang to me.  What this construct
> means is both input #5 and output #1 live in the same register, and that
> an `__u32' value is taken on input (from the result of the `htonl(proto)'
> calculation) and an `unsigned short' value produced in the same register
> on output, that'll be the value of the `proto' variable from there on.  A
> perfectly valid arrangement.  This would be the right arrangement to use
> with the MIPS16 SEH instruction for example.  Has this bug been reported
> to clang maintainers?

I'm not convinced it's a bug, but I do at least agree that the use case sounds
sensible. It makes sense to me that the focus should be on register allocations
rather than on types. However, the relevant clang source is being very specific
about the cases it is/isn't allowing which suggests it's deliberate. I've started a
thread on the clang mailing list to try to find out more about why we currently
reject it.

>  And I'd prefer to leave the declaration of `proto' alone as IPv6 network
> protocol numbers are 16-bit quantities.
>
>  That said this code is indeed weird if not wrong, which is probably why
> this arrangement resulted, in an attempt to prevent GCC from messing up
> the registers used.
> 
>  First and foremost both outputs, and especially #1, lack an earlyclobber.
> This I imagine may have prompted GCC to overwrite one of the inputs, which
> in turn is why whoever poked at this code decided to alias input #5 to
> output #1.  But as you can see in the asm there's no real aliasing between
> input #5 and output #1.  Input #5 is consumed early on (and even referred
> to with `%5' rather than `%1', which would be the norm in the case of
> actual aliasing), and the containing register reused for something else.
> So the two operands can be separated.  This is unlike input #4 vs output
> #0, that is both read and written right away (and just as one'd expect
> there's no reference to `%4' anywhere).
> 
>  Output #0 can do without an earlyclobber as it is aliased to input #4 and
> therefore cannot be assigned by GCC to another input.  But it won't hurt
> to have one too and it will set a good practice and serve a documentation
> purpose.
> 
>  I suggest a fix like this then:
> 
> static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
> 					  const struct in6_addr *daddr,
> 					  __u32 len, unsigned short proto,
> 					  __wsum sum)
> {
> 	__wsum tmp;
> 
> 	__asm__(
> [...]
> 	: "=&r" (sum), "=&r" (tmp)
> 	: "r" (saddr), "r" (daddr),
> 	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));
> 
>         return csum_fold(sum);
> }
> 
> Try and see if it works for you.
> 
>  I wonder why this is an asm in the first place though.  There's no rocket
> science here that GCC couldn't handle.  I guess it must have been very bad
> at optimising a C equivalent then.
> 
>   Maciej

Yes, that works for me on both GCC and Clang. I'll change the patch to this.
Would you like a 'Suggested-By' in the patch description?

RE: [PATCH 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Maciej W. Rozycki]

On Thu, 5 Feb 2015, Daniel Sanders wrote:

> Apologies for the slow response. I've had an excessive amount of 
> meetings in the last couple days.

 No worries, if anyone, it's not me in a hurry here. ;)

> >  This definitely looks like a bug in clang to me.  What this construct
> > means is both input #5 and output #1 live in the same register, and that
> > an `__u32' value is taken on input (from the result of the `htonl(proto)'
> > calculation) and an `unsigned short' value produced in the same register
> > on output, that'll be the value of the `proto' variable from there on.  A
> > perfectly valid arrangement.  This would be the right arrangement to use
> > with the MIPS16 SEH instruction for example.  Has this bug been reported
> > to clang maintainers?
> 
> I'm not convinced it's a bug, but I do at least agree that the use case sounds
> sensible. It makes sense to me that the focus should be on register allocations
> rather than on types. However, the relevant clang source is being very specific
> about the cases it is/isn't allowing which suggests it's deliberate. I've started a
> thread on the clang mailing list to try to find out more about why we currently
> reject it.

 I think it boils down to the register model implemented by a given 
architecture.  MIPS processors do not have subregisters for integer data 
quantities narrower than the size of the machine word.  The same GPR will 
hold a `char', a `short' or an `int', and therefore it is perfectly valid 
to arrange that it holds say an `int' on input and say a `short' on 
output.  So given an artificial example like this:

short foo(int i)
{
	short v;

	asm("frob %0" : "=r" (v) : "0" (i));
	return v;
}

the compiler knows it does not have to truncate the result of the 
calculation made in the asm before returning it to the caller, which it 
would unnecessarily have with this code:

short foo(int i)
{
	asm("frob %0" : "+r" (i));
	return i;
}

 This is unlike some other architectures.  On x86 for example you do have 
subregisters, so for example an `int' will be stored in %eax, a short will 
be stored in %ax, and a `char' will be stored in %al, or worse yet, %ah.  
Consequently you may not be able to alias an input operand and an output 
operand of a different width each to each other.

 Also I think it is important to note that in the (first) example above, 
`i' and `v' are separate data entities as far as C code is concerned.  
And that the operands of the asm merely tell the C compiler that the value 
of `i' must appear in a register (that need not be the variable's original 
storage location) on entry to the asm and the value to set `v' to will 
appear in a register (that again need not be the place where the actual 
variable lives) on exit from the asm, and that the two registers must be 
physically the same (presumably because of a machine limitation, such as 
one observed with the MIPS16 SEH instruction mentioned), but that does not 
mean the input and the output otherwise have anything in common.

 And last but not least, the extended asm feature is a GCC extension, so 
if clang developers chose to implement it for GCC compatibility, then I am 
afraid they need to follow the rules set by GCC.  So if GCC accepts it, 
they need too.

> Yes, that works for me on both GCC and Clang. I'll change the patch to this.
> Would you like a 'Suggested-By' in the patch description?

 Sure.

  Maciej

[PATCH v2 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Daniel Sanders]

Replace incorrect matching constraint that caused the error with an alternative
that still has the required constraints on the inline assembly.

This is the error message reported by clang:
arch/mips/include/asm/checksum.h:285:27: error: unsupported inline asm: input with type '__be32' (aka 'unsigned int') matching output with type 'unsigned short'
          "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
                                 ^~~~~~~~~~~~

The changed code can be compiled successfully by both gcc and clang.

Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Suggested-by: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org

---

Rewrote the patch following Maciej's suggestion where he observed that
the assembly was somewhat strange and suggested correcting the
constraints and using a local of matching type.

 arch/mips/include/asm/checksum.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
index 3418c51..48bfcaf 100644
--- a/arch/mips/include/asm/checksum.h
+++ b/arch/mips/include/asm/checksum.h
@@ -228,6 +228,8 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 					  __u32 len, unsigned short proto,
 					  __wsum sum)
 {
+        __wsum tmp;
+
 	__asm__(
 	"	.set	push		# csum_ipv6_magic\n"
 	"	.set	noreorder	\n"
@@ -280,9 +282,9 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 
 	"	addu	%0, $1		# Add final carry\n"
 	"	.set	pop"
-	: "=r" (sum), "=r" (proto)
+	: "=&r" (sum), "=&r" (tmp)
 	: "r" (saddr), "r" (daddr),
-	  "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
+	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));
 
 	return csum_fold(sum);
 }
-- 
2.1.4

Re: [PATCH v2 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Maciej W. Rozycki]

On Mon, 9 Feb 2015, Daniel Sanders wrote:

> --- a/arch/mips/include/asm/checksum.h
> +++ b/arch/mips/include/asm/checksum.h
> @@ -228,6 +228,8 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
>  					  __u32 len, unsigned short proto,
>  					  __wsum sum)
>  {
> +        __wsum tmp;

 Formatting issue here, use a tab.

  Maciej

[PATCH v3 3/5] MIPS: LLVMLinux: Fix an 'inline asm input/output type mismatch' error.

[Daniel Sanders]

Replace incorrect matching constraint that caused the error with an alternative
that still has the required constraints on the inline assembly.

This is the error message reported by clang:
arch/mips/include/asm/checksum.h:285:27: error: unsupported inline asm: input with type '__be32' (aka 'unsigned int') matching output with type 'unsigned short'
          "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
                                 ^~~~~~~~~~~~

The changed code can be compiled successfully by both gcc and clang.

Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Suggested-by: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org

---

Rewrote the patch following Maciej's suggestion where he observed that
the assembly was somewhat strange and suggested correcting the
constraints and using a local of matching type.

Fixed a silly whitespace mistake that was introduced in v2.

 arch/mips/include/asm/checksum.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
index 3418c51..ebad4f4 100644
--- a/arch/mips/include/asm/checksum.h
+++ b/arch/mips/include/asm/checksum.h
@@ -228,6 +228,8 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 					  __u32 len, unsigned short proto,
 					  __wsum sum)
 {
+	__wsum tmp;
+
 	__asm__(
 	"	.set	push		# csum_ipv6_magic\n"
 	"	.set	noreorder	\n"
@@ -280,9 +282,9 @@ static __inline__ __sum16 csum_ipv6_magic(const struct in6_addr *saddr,
 
 	"	addu	%0, $1		# Add final carry\n"
 	"	.set	pop"
-	: "=r" (sum), "=r" (proto)
+	: "=&r" (sum), "=&r" (tmp)
 	: "r" (saddr), "r" (daddr),
-	  "0" (htonl(len)), "1" (htonl(proto)), "r" (sum));
+	  "0" (htonl(len)), "r" (htonl(proto)), "r" (sum));
 
 	return csum_fold(sum);
 }
-- 
2.1.4

[PATCH 4/5] MIPS: LLVMLinux: Silence variable self-assignment warnings.

[Daniel Sanders]

From: Toma Tabacu <toma.tabacu@imgtec.com>

Remove variable self-assignments.
This silences a bunch of -Wself-assign warnings reported by clang.
The changed code can be compiled without warnings by both gcc and clang.

Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/math-emu/dp_add.c | 5 -----
 arch/mips/math-emu/dp_sub.c | 5 -----
 arch/mips/math-emu/sp_add.c | 5 -----
 arch/mips/math-emu/sp_sub.c | 5 -----
 4 files changed, 20 deletions(-)

diff --git a/arch/mips/math-emu/dp_add.c b/arch/mips/math-emu/dp_add.c
index 7f64577..58b2795 100644
--- a/arch/mips/math-emu/dp_add.c
+++ b/arch/mips/math-emu/dp_add.c
@@ -150,8 +150,6 @@ union ieee754dp ieee754dp_add(union ieee754dp x, union ieee754dp y)
 		 * leaving result in xm, xs and xe.
 		 */
 		xm = xm + ym;
-		xe = xe;
-		xs = xs;
 
 		if (xm >> (DP_FBITS + 1 + 3)) { /* carry out */
 			xm = XDPSRS1(xm);
@@ -160,11 +158,8 @@ union ieee754dp ieee754dp_add(union ieee754dp x, union ieee754dp y)
 	} else {
 		if (xm >= ym) {
 			xm = xm - ym;
-			xe = xe;
-			xs = xs;
 		} else {
 			xm = ym - xm;
-			xe = xe;
 			xs = ys;
 		}
 		if (xm == 0)
diff --git a/arch/mips/math-emu/dp_sub.c b/arch/mips/math-emu/dp_sub.c
index 7a17402..2eb87cd2 100644
--- a/arch/mips/math-emu/dp_sub.c
+++ b/arch/mips/math-emu/dp_sub.c
@@ -153,8 +153,6 @@ union ieee754dp ieee754dp_sub(union ieee754dp x, union ieee754dp y)
 		/* generate 28 bit result of adding two 27 bit numbers
 		 */
 		xm = xm + ym;
-		xe = xe;
-		xs = xs;
 
 		if (xm >> (DP_FBITS + 1 + 3)) { /* carry out */
 			xm = XDPSRS1(xm);	/* shift preserving sticky */
@@ -163,11 +161,8 @@ union ieee754dp ieee754dp_sub(union ieee754dp x, union ieee754dp y)
 	} else {
 		if (xm >= ym) {
 			xm = xm - ym;
-			xe = xe;
-			xs = xs;
 		} else {
 			xm = ym - xm;
-			xe = xe;
 			xs = ys;
 		}
 		if (xm == 0) {
diff --git a/arch/mips/math-emu/sp_add.c b/arch/mips/math-emu/sp_add.c
index 2d84d46..7a33af4 100644
--- a/arch/mips/math-emu/sp_add.c
+++ b/arch/mips/math-emu/sp_add.c
@@ -148,8 +148,6 @@ union ieee754sp ieee754sp_add(union ieee754sp x, union ieee754sp y)
 		 * leaving result in xm, xs and xe.
 		 */
 		xm = xm + ym;
-		xe = xe;
-		xs = xs;
 
 		if (xm >> (SP_FBITS + 1 + 3)) { /* carry out */
 			SPXSRSX1();
@@ -157,11 +155,8 @@ union ieee754sp ieee754sp_add(union ieee754sp x, union ieee754sp y)
 	} else {
 		if (xm >= ym) {
 			xm = xm - ym;
-			xe = xe;
-			xs = xs;
 		} else {
 			xm = ym - xm;
-			xe = xe;
 			xs = ys;
 		}
 		if (xm == 0)
diff --git a/arch/mips/math-emu/sp_sub.c b/arch/mips/math-emu/sp_sub.c
index 8592e49..1189bc5 100644
--- a/arch/mips/math-emu/sp_sub.c
+++ b/arch/mips/math-emu/sp_sub.c
@@ -148,8 +148,6 @@ union ieee754sp ieee754sp_sub(union ieee754sp x, union ieee754sp y)
 		/* generate 28 bit result of adding two 27 bit numbers
 		 */
 		xm = xm + ym;
-		xe = xe;
-		xs = xs;
 
 		if (xm >> (SP_FBITS + 1 + 3)) { /* carry out */
 			SPXSRSX1();	/* shift preserving sticky */
@@ -157,11 +155,8 @@ union ieee754sp ieee754sp_sub(union ieee754sp x, union ieee754sp y)
 	} else {
 		if (xm >= ym) {
 			xm = xm - ym;
-			xe = xe;
-			xs = xs;
 		} else {
 			xm = ym - xm;
-			xe = xe;
 			xs = ys;
 		}
 		if (xm == 0) {
-- 
2.1.4

[PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

[Daniel Sanders]

From: Toma Tabacu <toma.tabacu@imgtec.com>

Differentiate the 'u' GAS .macro argument from the '\u' C preprocessor unicode
escape sequence by renaming it to '_u'.

When the 'u' argument is evaluated, we end up writing '\u', which causes
clang to emit -Wunicode warnings.

This silences a couple of -Wunicode warnings reported by clang.
The changed code can be preprocessed without warnings by both gcc and clang.

Signed-off-by: Toma Tabacu <toma.tabacu@imgtec.com>
Signed-off-by: Daniel Sanders <daniel.sanders@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Paul Burton <paul.burton@imgtec.com>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: "Steven J. Hill" <Steven.Hill@imgtec.com>
Cc: Manuel Lauss <manuel.lauss@gmail.com>
Cc: Jim Quinlan <jim2101024@gmail.com>
Cc: linux-mips@linux-mips.org
Cc: linux-kernel@vger.kernel.org
---
 arch/mips/include/asm/asmmacro.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/asmmacro.h b/arch/mips/include/asm/asmmacro.h
index 6caf876..c657932 100644
--- a/arch/mips/include/asm/asmmacro.h
+++ b/arch/mips/include/asm/asmmacro.h
@@ -200,12 +200,12 @@
 	.word	0x41600021 | (\reg << 16)
 	.endm
 
-	.macro	MFTR	rt=0, rd=0, u=0, sel=0
-	 .word	0x41000000 | (\rt << 16) | (\rd << 11) | (\u << 5) | (\sel)
+	.macro	MFTR	rt=0, rd=0, _u=0, sel=0
+	 .word	0x41000000 | (\rt << 16) | (\rd << 11) | (\_u << 5) | (\sel)
 	.endm
 
-	.macro	MTTR	rt=0, rd=0, u=0, sel=0
-	 .word	0x41800000 | (\rt << 16) | (\rd << 11) | (\u << 5) | (\sel)
+	.macro	MTTR	rt=0, rd=0, _u=0, sel=0
+	 .word	0x41800000 | (\rt << 16) | (\rd << 11) | (\_u << 5) | (\sel)
 	.endm
 
 #ifdef TOOLCHAIN_SUPPORTS_MSA
-- 
2.1.4

Re: [PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

[Maciej W. Rozycki]

On Tue, 3 Feb 2015, Daniel Sanders wrote:

> From: Toma Tabacu <toma.tabacu@imgtec.com>
> 
> Differentiate the 'u' GAS .macro argument from the '\u' C preprocessor unicode
> escape sequence by renaming it to '_u'.
> 
> When the 'u' argument is evaluated, we end up writing '\u', which causes
> clang to emit -Wunicode warnings.
> 
> This silences a couple of -Wunicode warnings reported by clang.
> The changed code can be preprocessed without warnings by both gcc and clang.

 I suspect it is a clang preprocessor bug that:

1. It interprets these character pairs as unicode escapes for assembly 
   sources, I think it should be up to the language translator rather 
   than the preprocessor, i.e. the assembler in this case (the notion of 
   unicode escapes for the preprocessor appears to be limited to 
   stringification, and then it is implementation-defined).

2. It considers these character pairs to be unicode escapes in the first 
   place given that they do not follow the syntax required for such 
   escapes, that is `\unnnn', where `n' are hex digits.

Of course it may be reasonable for us to work this bug around as we've 
been doing for years with GCC, but has the issue been reported back to 
clang maintainers?  What was their response?

  Maciej

RE: [PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

[Toma Tabacu]

On Wed, 4 Feb 2015, Maciej W. Rozycki wrote:
> 2. It considers these character pairs to be unicode escapes in the first 
>    place given that they do not follow the syntax required for such 
>    escapes, that is `\unnnn', where `n' are hex digits.
> 

It doesn't actually treat them as unicode escapes, but it still warns the user,
in case they were meant to be unicode escapes. Here's the warning message:

arch/mips/include/asm/asmmacro.h:197:51: warning: \u used with no following hex digits; treating as '\' followed by identifier [-Wunicode]
         .word  0x41000000 | (\rt << 16) | (\rd << 11) | (\u << 5) | (\sel)
                                                          ^
I'll add it to the summary in v2.

> Of course it may be reasonable for us to work this bug around as we've 
> been doing for years with GCC, but has the issue been reported back to 
> clang maintainers?  What was their response?
> 

It hasn't been reported, but I don't think they would agree with removing
unicode escape sequences from the assembler-with-cpp mode because it is
currently being used for other languages as well, not just assembly.

One such language is Haskell (ghc, to be more specific), for which the clang
developers had to actually stop the preprocessor from enforcing the C universal
character name restrictions in assembler-with-cpp mode, which suggests that ghc
wants the preprocessor to check for unicode escape sequences.

At the moment, we can either disable -Wunicode for asmmacro.h or refrain from
using '\u' as an identifier.

RE: [PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

[Maciej W. Rozycki]

On Thu, 5 Feb 2015, Toma Tabacu wrote:

> > 2. It considers these character pairs to be unicode escapes in the first 
> >    place given that they do not follow the syntax required for such 
> >    escapes, that is `\unnnn', where `n' are hex digits.
> > 
> 
> It doesn't actually treat them as unicode escapes, but it still warns the user,
> in case they were meant to be unicode escapes. Here's the warning message:
> 
> arch/mips/include/asm/asmmacro.h:197:51: warning: \u used with no following hex digits; treating as '\' followed by identifier [-Wunicode]
>          .word  0x41000000 | (\rt << 16) | (\rd << 11) | (\u << 5) | (\sel)
>                                                           ^
> I'll add it to the summary in v2.

 Thanks, that makes things clearer.  It always makes sense to include the 
exact error message produced where applicable or otherwise people do not 
necessarily know what the matter is.

> > Of course it may be reasonable for us to work this bug around as we've 
> > been doing for years with GCC, but has the issue been reported back to 
> > clang maintainers?  What was their response?
> > 
> 
> It hasn't been reported, but I don't think they would agree with removing
> unicode escape sequences from the assembler-with-cpp mode because it is
> currently being used for other languages as well, not just assembly.

 First, preprocessing rules surely have to be language specific.  The C 
language standard does not specify what the preprocessor is meant to do 
(if anything) for other languages.  GCC or clang -- that's no different.  

 The assembly language has a different syntax and `\u' has a different 
meaning in the context of assembly macro expansion than it would have in a 
name of a symbol, where such a Unicode escape sequence might indeed be 
interpreted as such and character encoded propagated to the symbol 
produced.  But that's up to the assembler -- GAS for example does not 
AFAIK support Unicode escape sequences in symbol names right now, but I 
suppose such a feature could be added if desired.

 Which prompts another question of course: how does the clang C compiler 
represent Unicode characters in identifiers in its assembly output?

 I have looked into the C language standard and it appears to me like the 
translation phase to interpret universal character names at has not been 
defined.  This is probably why the standard does specify the result of 
pasting preprocessor tokens together as undefined if a universal character 
name is produced this way.

 Consequently I think an important question in this context is: does 
clang's preprocessor actually convert these sequences anyhow before 
passing them down to the compiler?  How for example does C output from a 
trivial example that contains such Unicode escape sequences look like 
then?

> One such language is Haskell (ghc, to be more specific), for which the clang
> developers had to actually stop the preprocessor from enforcing the C universal
> character name restrictions in assembler-with-cpp mode, which suggests that ghc
> wants the preprocessor to check for unicode escape sequences.
> 
> At the moment, we can either disable -Wunicode for asmmacro.h or refrain from
> using '\u' as an identifier.

 To be clear: it's `u' here that is the identifier, the leading `\' is 
merely how assembly syntax has been specified for references to macro 
arguments.  And TBH I find banning any macro arguments starting with `u' 
rather silly.  I'm leaning towards considering having -Wunicode disabled 
for all assembly sources, or maybe even for the whole Linux compilation, 
the right solution.  It's not like we have a need for Unicode identifiers.  

 What's the exact semantics of -Wunicode for clang?

  Maciej

Re: [PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

[Måns Rullgård]

"Maciej W. Rozycki" <macro@linux-mips.org> writes:

> On Thu, 5 Feb 2015, Toma Tabacu wrote:
>
>> > 2. It considers these character pairs to be unicode escapes in the first 
>> >    place given that they do not follow the syntax required for such 
>> >    escapes, that is `\unnnn', where `n' are hex digits.
>> > 
>> 
>> It doesn't actually treat them as unicode escapes, but it still warns
>> the user, in case they were meant to be unicode escapes. Here's the
>> warning message:
>> 
>> arch/mips/include/asm/asmmacro.h:197:51: warning: \u used with no following hex digits; treating as '\' followed by identifier [-Wunicode]
>>          .word  0x41000000 | (\rt << 16) | (\rd << 11) | (\u << 5) | (\sel)
>>                                                           ^
>> I'll add it to the summary in v2.
>
>  Thanks, that makes things clearer.  It always makes sense to include the 
> exact error message produced where applicable or otherwise people do not 
> necessarily know what the matter is.
>
>> > Of course it may be reasonable for us to work this bug around as we've 
>> > been doing for years with GCC, but has the issue been reported back to 
>> > clang maintainers?  What was their response?
>> > 
>> 
>> It hasn't been reported, but I don't think they would agree with removing
>> unicode escape sequences from the assembler-with-cpp mode because it is
>> currently being used for other languages as well, not just assembly.
>
>  First, preprocessing rules surely have to be language specific.  The C 
> language standard does not specify what the preprocessor is meant to do 
> (if anything) for other languages.  GCC or clang -- that's no different.  
>
>  The assembly language has a different syntax and `\u' has a different 
> meaning in the context of assembly macro expansion than it would have in a 
> name of a symbol, where such a Unicode escape sequence might indeed be 
> interpreted as such and character encoded propagated to the symbol 
> produced.  But that's up to the assembler -- GAS for example does not 
> AFAIK support Unicode escape sequences in symbol names right now, but I 
> suppose such a feature could be added if desired.
>
>  Which prompts another question of course: how does the clang C compiler 
> represent Unicode characters in identifiers in its assembly output?
>
>  I have looked into the C language standard and it appears to me like the 
> translation phase to interpret universal character names at has not been 
> defined.  This is probably why the standard does specify the result of 
> pasting preprocessor tokens together as undefined if a universal character 
> name is produced this way.

That is my interpretation as well.

>  Consequently I think an important question in this context is: does 
> clang's preprocessor actually convert these sequences anyhow before 
> passing them down to the compiler?  How for example does C output from a 
> trivial example that contains such Unicode escape sequences look like 
> then?
>
>> One such language is Haskell (ghc, to be more specific), for which
>> the clang developers had to actually stop the preprocessor from
>> enforcing the C universal character name restrictions in
>> assembler-with-cpp mode, which suggests that ghc wants the
>> preprocessor to check for unicode escape sequences.
>> 
>> At the moment, we can either disable -Wunicode for asmmacro.h or
>> refrain from using '\u' as an identifier.
>
>  To be clear: it's `u' here that is the identifier, the leading `\' is 
> merely how assembly syntax has been specified for references to macro 
> arguments.  And TBH I find banning any macro arguments starting with `u' 
> rather silly.

Agreed.

> I'm leaning towards considering having -Wunicode disabled for all
> assembly sources, or maybe even for the whole Linux compilation, the
> right solution.  It's not like we have a need for Unicode identifiers.

It might be an idea to disable -Wunicode and have checkpatch warn about
Unicode escapes instead if people are worried about this.  Personally, I
doubt there's much cause for concern here.

-- 
Måns Rullgård
mans@mansr.com

RE: [PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when preprocessing assembly.

[Daniel Sanders]

Apologies for the slow reply.

> -----Original Message-----
> From: Måns Rullgård [mailto:mans@mansr.com]
> Sent: 05 February 2015 12:56
> To: Maciej W. Rozycki
> Cc: Toma Tabacu; Daniel Sanders; Ralf Baechle; Paul Burton; Paul Bolle;
> Steven J. Hill; Manuel Lauss; Jim Quinlan; linux-mips@linux-mips.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 5/5] MIPS: LLVMLinux: Silence unicode warnings when
> preprocessing assembly.
> 
> "Maciej W. Rozycki" <macro@linux-mips.org> writes:
> 
> > On Thu, 5 Feb 2015, Toma Tabacu wrote:
> >
> >> > 2. It considers these character pairs to be unicode escapes in the first
> >> >    place given that they do not follow the syntax required for such
> >> >    escapes, that is `\unnnn', where `n' are hex digits.
> >> >
> >>
> >> It doesn't actually treat them as unicode escapes, but it still warns
> >> the user, in case they were meant to be unicode escapes. Here's the
> >> warning message:
> >>
> >> arch/mips/include/asm/asmmacro.h:197:51: warning: \u used with no
> following hex digits; treating as '\' followed by identifier [-Wunicode]
> >>          .word  0x41000000 | (\rt << 16) | (\rd << 11) | (\u << 5) | (\sel)
> >>                                                           ^
> >> I'll add it to the summary in v2.
> >
> >  Thanks, that makes things clearer.  It always makes sense to include the
> > exact error message produced where applicable or otherwise people do not
> > necessarily know what the matter is.
> >
> >> > Of course it may be reasonable for us to work this bug around as we've
> >> > been doing for years with GCC, but has the issue been reported back to
> >> > clang maintainers?  What was their response?
> >> >
> >>
> >> It hasn't been reported, but I don't think they would agree with removing
> >> unicode escape sequences from the assembler-with-cpp mode because it is
> >> currently being used for other languages as well, not just assembly.
> >
> >  First, preprocessing rules surely have to be language specific.  The C
> > language standard does not specify what the preprocessor is meant to do
> > (if anything) for other languages.  GCC or clang -- that's no different.
> > 
> >  The assembly language has a different syntax and `\u' has a different
> > meaning in the context of assembly macro expansion than it would have in a
> > name of a symbol, where such a Unicode escape sequence might indeed be
> > interpreted as such and character encoded propagated to the symbol
> > produced.  But that's up to the assembler -- GAS for example does not
> > AFAIK support Unicode escape sequences in symbol names right now, but I
> > suppose such a feature could be added if desired.

Pre-processed assembly is somewhat unusual in that it has traditionally been
pre-processed with a pre-processor designed for the C language. It's certainly
possible to have assembly specific tweaks (GCC has a couple) but it is still a C
pre-processor at heart. It doesn't know anything about the assembly language,
it just happens to be similar enough to be usable.

>From the pre-processors point of view, '\u' is two pre-processor tokens '\' and
the identifier 'u'. However, with following hex digits it would have been an identifier
starting with a universal character name. Clang's warning is effectively saying that
the former is more likely to be the intention. That's probably not as true for
pre-processed assembly as it is for C/C++.

> >  Which prompts another question of course: how does the clang C compiler
> > represent Unicode characters in identifiers in its assembly output?

They're emitted as multi-byte characters.

> >  I have looked into the C language standard and it appears to me like the
> > translation phase to interpret universal character names at has not been
> > defined.  This is probably why the standard does specify the result of
> > pasting preprocessor tokens together as undefined if a universal character
> > name is produced this way.
> 
> That is my interpretation as well.

It's my understanding that they should be interpreted when pre-processing tokens
are formed. This is based on the fact that the universal character names are included in
the grammar for identifiers and are not discussed in a separate translation phase.
I agree that it doesn't explicitly state that though.

> >  Consequently I think an important question in this context is: does
> > clang's preprocessor actually convert these sequences anyhow before
> > passing them down to the compiler?  How for example does C output from a
> > trivial example that contains such Unicode escape sequences look like
> > then?

Clang is converting them to multibyte characters during pre-processing.

> >> One such language is Haskell (ghc, to be more specific), for which
> >> the clang developers had to actually stop the preprocessor from
> >> enforcing the C universal character name restrictions in
> >> assembler-with-cpp mode, which suggests that ghc wants the
> >> preprocessor to check for unicode escape sequences.
> >>
> >> At the moment, we can either disable -Wunicode for asmmacro.h or
> >> refrain from using '\u' as an identifier.
> >
> >  To be clear: it's `u' here that is the identifier, the leading `\' is
> > merely how assembly syntax has been specified for references to macro
> > arguments.  And TBH I find banning any macro arguments starting with `u'
> > rather silly.
> 
> Agreed.

That's the crux of the issue. Had it been followed by some hex-digits,
it would be an identifier '\u1234' and not a '\' followed by the identifier 'u'.
Clang currently thinks the former is more likely and warns.

I do agree that warning about all macro arguments beginning with 'u' is silly though.
Perhaps for assembler-with-cpp mode the warning should be suppressed when
it's the first character of an identifier.

> > I'm leaning towards considering having -Wunicode disabled for all
> > assembly sources, or maybe even for the whole Linux compilation, the
> > right solution.  It's not like we have a need for Unicode identifiers.
> 
> It might be an idea to disable -Wunicode and have checkpatch warn about
> Unicode escapes instead if people are worried about this.  Personally, I
> doubt there's much cause for concern here.
> 
> --
> Måns Rullgård
> mans@mansr.com

I'm fine with disabling -Wunicode if that's our preferred solution.