Hi,
with GCC 10 there is issue with simple memset implementation which is
called recursively. There are couple of discussions about it and the first
two patches are trying to workaround this.
The third patch only removes simple implementations from arch code and use
generic one which is the same.
Thanks,
Michal
I sent only 1 patch in v1 that's why sending v2 with all 3.
Changes in v2:
- missing patch in v1
- missing patch in v1
Michal Simek (3):
microblaze: Use simple memset implementation from lib/string.c
microblaze: Do loop unrolling for optimized memset implementation
microblaze: Use simple memmove/memcpy implementation from lib/string.c
arch/microblaze/include/asm/string.h | 2 ++
arch/microblaze/lib/memcpy.c | 18 ++-------------
arch/microblaze/lib/memmove.c | 29 ++----------------------
arch/microblaze/lib/memset.c | 33 ++++++++++++----------------
4 files changed, 20 insertions(+), 62 deletions(-)
--
2.35.1
This is based on previous commit ("microblaze: Use simple memset
implementation from lib/string.c") where generic memset implementation is
used when OPT_LIB_FUNCTION is not defined. The same change can be done for
memset/memcpy implementation where doesn't make sense to have generic
implementation in architecture code.
Signed-off-by: Michal Simek <[email protected]>
---
(no changes since v1)
arch/microblaze/include/asm/string.h | 2 +-
arch/microblaze/lib/memcpy.c | 18 ++---------------
arch/microblaze/lib/memmove.c | 29 ++--------------------------
3 files changed, 5 insertions(+), 44 deletions(-)
diff --git a/arch/microblaze/include/asm/string.h b/arch/microblaze/include/asm/string.h
index dbdb9eb4a733..8798ad2c132a 100644
--- a/arch/microblaze/include/asm/string.h
+++ b/arch/microblaze/include/asm/string.h
@@ -10,13 +10,13 @@
#ifdef CONFIG_OPT_LIB_FUNCTION
#define __HAVE_ARCH_MEMSET
-#endif
#define __HAVE_ARCH_MEMCPY
#define __HAVE_ARCH_MEMMOVE
extern void *memset(void *, int, __kernel_size_t);
extern void *memcpy(void *, const void *, __kernel_size_t);
extern void *memmove(void *, const void *, __kernel_size_t);
+#endif
#endif /* __KERNEL__ */
diff --git a/arch/microblaze/lib/memcpy.c b/arch/microblaze/lib/memcpy.c
index 63041fdf916d..9966dce55619 100644
--- a/arch/microblaze/lib/memcpy.c
+++ b/arch/microblaze/lib/memcpy.c
@@ -31,20 +31,7 @@
#include <linux/string.h>
-#ifdef __HAVE_ARCH_MEMCPY
-#ifndef CONFIG_OPT_LIB_FUNCTION
-void *memcpy(void *v_dst, const void *v_src, __kernel_size_t c)
-{
- const char *src = v_src;
- char *dst = v_dst;
-
- /* Simple, byte oriented memcpy. */
- while (c--)
- *dst++ = *src++;
-
- return v_dst;
-}
-#else /* CONFIG_OPT_LIB_FUNCTION */
+#ifdef CONFIG_OPT_LIB_FUNCTION
void *memcpy(void *v_dst, const void *v_src, __kernel_size_t c)
{
const char *src = v_src;
@@ -188,6 +175,5 @@ void *memcpy(void *v_dst, const void *v_src, __kernel_size_t c)
return v_dst;
}
-#endif /* CONFIG_OPT_LIB_FUNCTION */
EXPORT_SYMBOL(memcpy);
-#endif /* __HAVE_ARCH_MEMCPY */
+#endif /* CONFIG_OPT_LIB_FUNCTION */
diff --git a/arch/microblaze/lib/memmove.c b/arch/microblaze/lib/memmove.c
index 9862f6b1e59d..2e49d0ef1e07 100644
--- a/arch/microblaze/lib/memmove.c
+++ b/arch/microblaze/lib/memmove.c
@@ -30,31 +30,7 @@
#include <linux/compiler.h>
#include <linux/string.h>
-#ifdef __HAVE_ARCH_MEMMOVE
-#ifndef CONFIG_OPT_LIB_FUNCTION
-void *memmove(void *v_dst, const void *v_src, __kernel_size_t c)
-{
- const char *src = v_src;
- char *dst = v_dst;
-
- if (!c)
- return v_dst;
-
- /* Use memcpy when source is higher than dest */
- if (v_dst <= v_src)
- return memcpy(v_dst, v_src, c);
-
- /* copy backwards, from end to beginning */
- src += c;
- dst += c;
-
- /* Simple, byte oriented memmove. */
- while (c--)
- *--dst = *--src;
-
- return v_dst;
-}
-#else /* CONFIG_OPT_LIB_FUNCTION */
+#ifdef CONFIG_OPT_LIB_FUNCTION
void *memmove(void *v_dst, const void *v_src, __kernel_size_t c)
{
const char *src = v_src;
@@ -215,6 +191,5 @@ void *memmove(void *v_dst, const void *v_src, __kernel_size_t c)
}
return v_dst;
}
-#endif /* CONFIG_OPT_LIB_FUNCTION */
EXPORT_SYMBOL(memmove);
-#endif /* __HAVE_ARCH_MEMMOVE */
+#endif /* CONFIG_OPT_LIB_FUNCTION */
--
2.35.1
On microblaze systems which are not using OPT_LIB_FUNCTION only simple
memset is used. This function is already implemented in lib/string.c that's
why it should be used instead.
This change is done in respect of issue fixed by commit 33d0f96ffd73
("lib/string.c: Use freestanding environment") where gcc-10.x moved
-ftree-loop-distribute-patterns optimization is to O2 optimization level.
This optimization causes GCC to convert the while loop in memset.c into a
call to memset. So This optimization is transforming a loop in a
memset/memcpy into a call to the function itself. This makes the memset
implementation as recursive.
Based on fix above -ffreestanding was used and it needs to be used on
Microblaze too but the patch is not adding this flag it removes simple
implementation to cause that generic implementation is used where this flag
is already setup.
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Mahesh Bodapati <[email protected]>
---
Changes in v2:
- missing patch in v1
arch/microblaze/include/asm/string.h | 2 ++
arch/microblaze/lib/memset.c | 20 ++------------------
2 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/arch/microblaze/include/asm/string.h b/arch/microblaze/include/asm/string.h
index 34071a848b6a..dbdb9eb4a733 100644
--- a/arch/microblaze/include/asm/string.h
+++ b/arch/microblaze/include/asm/string.h
@@ -8,7 +8,9 @@
#ifdef __KERNEL__
+#ifdef CONFIG_OPT_LIB_FUNCTION
#define __HAVE_ARCH_MEMSET
+#endif
#define __HAVE_ARCH_MEMCPY
#define __HAVE_ARCH_MEMMOVE
diff --git a/arch/microblaze/lib/memset.c b/arch/microblaze/lib/memset.c
index eb6c8988af02..615a2f8f53cb 100644
--- a/arch/microblaze/lib/memset.c
+++ b/arch/microblaze/lib/memset.c
@@ -30,22 +30,7 @@
#include <linux/compiler.h>
#include <linux/string.h>
-#ifdef __HAVE_ARCH_MEMSET
-#ifndef CONFIG_OPT_LIB_FUNCTION
-void *memset(void *v_src, int c, __kernel_size_t n)
-{
- char *src = v_src;
-
- /* Truncate c to 8 bits */
- c = (c & 0xFF);
-
- /* Simple, byte oriented memset or the rest of count. */
- while (n--)
- *src++ = c;
-
- return v_src;
-}
-#else /* CONFIG_OPT_LIB_FUNCTION */
+#ifdef CONFIG_OPT_LIB_FUNCTION
void *memset(void *v_src, int c, __kernel_size_t n)
{
char *src = v_src;
@@ -94,6 +79,5 @@ void *memset(void *v_src, int c, __kernel_size_t n)
return v_src;
}
-#endif /* CONFIG_OPT_LIB_FUNCTION */
EXPORT_SYMBOL(memset);
-#endif /* __HAVE_ARCH_MEMSET */
+#endif /* CONFIG_OPT_LIB_FUNCTION */
--
2.35.1
pá 25. 2. 2022 v 14:55 odesílatel Michal Simek <[email protected]> napsal:
>
> Hi,
>
> with GCC 10 there is issue with simple memset implementation which is
> called recursively. There are couple of discussions about it and the first
> two patches are trying to workaround this.
> The third patch only removes simple implementations from arch code and use
> generic one which is the same.
>
> Thanks,
> Michal
>
> I sent only 1 patch in v1 that's why sending v2 with all 3.
>
>
> Changes in v2:
> - missing patch in v1
> - missing patch in v1
>
> Michal Simek (3):
> microblaze: Use simple memset implementation from lib/string.c
> microblaze: Do loop unrolling for optimized memset implementation
> microblaze: Use simple memmove/memcpy implementation from lib/string.c
>
> arch/microblaze/include/asm/string.h | 2 ++
> arch/microblaze/lib/memcpy.c | 18 ++-------------
> arch/microblaze/lib/memmove.c | 29 ++----------------------
> arch/microblaze/lib/memset.c | 33 ++++++++++++----------------
> 4 files changed, 20 insertions(+), 62 deletions(-)
>
> --
> 2.35.1
>
Applied.
M
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs