[PATCH (sh-2.6)] sh: Use GCC __builtin_prefetch() to implement prefetch (V2)

[Giuseppe CAVALLARO]

GCC's __builtin_prefetch() was introduced a long time ago, all
supported GCC versions have it.
So this patch removes the ARCH_HAS_PREFETCH and ARCH_HAS_PREFETCHW
macros, defined for SH2A and SH4 that will fall back on
__builtin_prefetch() through the generic code:
include/linux/prefetch.h

The builtin usage should be more efficient that an __asm__
because less barriers, and because the compiler doesn't see the
inst as a "black box" allowing better code generation.
Many thanks to Christian Bruel <***@st.com> for his
support on evaluate the impact of the gcc built-in on SH4 arch.

Signed-off-by: Giuseppe Cavallaro <***@st.com>
Signed-off-by: Stuart Menefy <***@st.com>
---
arch/sh/include/asm/processor_32.h | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/arch/sh/include/asm/processor_32.h b/arch/sh/include/asm/processor_32.h
index 46d5179..37417eb 100644
--- a/arch/sh/include/asm/processor_32.h
+++ b/arch/sh/include/asm/processor_32.h
@@ -195,14 +195,6 @@ extern unsigned long get_wchan(struct task_struct *p);

#if defined(CONFIG_CPU_SH2A) || defined(CONFIG_CPU_SH4)
#define PREFETCH_STRIDE L1_CACHE_BYTES
-#define ARCH_HAS_PREFETCH
-#define ARCH_HAS_PREFETCHW
-static inline void prefetch(void *x)
-{
- __asm__ __volatile__ ("pref @%0\n\t" : : "r" (x) : "memory");
-}
-
-#define prefetchw(x) prefetch(x)
#endif

#endif /* __KERNEL__ */

[Paul Mundt]

Post by Giuseppe CAVALLARO
GCC's __builtin_prefetch() was introduced a long time ago, all
supported GCC versions have it.
So this patch removes the ARCH_HAS_PREFETCH and ARCH_HAS_PREFETCHW
macros, defined for SH2A and SH4 that will fall back on
include/linux/prefetch.h
The builtin usage should be more efficient that an __asm__
because less barriers, and because the compiler doesn't see the
inst as a "black box" allowing better code generation.
support on evaluate the impact of the gcc built-in on SH4 arch.

Actually now that I think about it, it's not that simple. If
ARCH_HAS_PREFETCH goes away then we lose prefetch_range() (which
admittely isn't called anywhere that matters, but it may in the future).

If gcc is smart enough to optimize out __builtin_prefetch() for the cases
where nothing has to be done, then the ARCH_HAS_PREFETCH define could
simply be killed and each arch would be responsible for establishing the
prefetch stride (this seems to vary between the size of an L1 or L2
cacheline depending on the platform). This is a different change though,
and is something you would have to bring up on linux-arch and
linux-kernel as a separate patch.

Perhaps the easiest solution for now is just to stick with your first
version, which at least retains the stride data and prefetch_range()
behaviour. If you wish to sort out the PREFETCH_STRIDE/ARCH_HAS_PREFETCH
and prefetch_range() mess separately then of course that's something we
can deal with incrementally, too.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Peppe CAVALLARO]

Hello Paul,

Post by Paul Mundt
Actually now that I think about it, it's not that simple. If
ARCH_HAS_PREFETCH goes away then we lose prefetch_range() (which
admittely isn't called anywhere that matters, but it may in the future).
If gcc is smart enough to optimize out __builtin_prefetch() for the cases
where nothing has to be done, then the ARCH_HAS_PREFETCH define could
simply be killed and each arch would be responsible for establishing the
prefetch stride (this seems to vary between the size of an L1 or L2
cacheline depending on the platform). This is a different change though,
and is something you would have to bring up on linux-arch and
linux-kernel as a separate patch.
Perhaps the easiest solution for now is just to stick with your first
version, which at least retains the stride data and prefetch_range()
behaviour.

Maybe it's worth having my first patch.
We don't lose the prefetch_range behavior.
Compiler should also be smart enough to manage the pref
inst optimizing the generated code.

Post by Paul Mundt
If you wish to sort out the PREFETCH_STRIDE/ARCH_HAS_PREFETCH
and prefetch_range() mess separately then of course that's something we
can deal with incrementally, too.

I agree, we can deal with that as step further.

Regards
Peppe
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Mundt]

Post by Peppe CAVALLARO

Post by Paul Mundt
Actually now that I think about it, it's not that simple. If
ARCH_HAS_PREFETCH goes away then we lose prefetch_range() (which
admittely isn't called anywhere that matters, but it may in the future).
If gcc is smart enough to optimize out __builtin_prefetch() for the cases
where nothing has to be done, then the ARCH_HAS_PREFETCH define could
simply be killed and each arch would be responsible for establishing the
prefetch stride (this seems to vary between the size of an L1 or L2
cacheline depending on the platform). This is a different change though,
and is something you would have to bring up on linux-arch and
linux-kernel as a separate patch.
Perhaps the easiest solution for now is just to stick with your first
version, which at least retains the stride data and prefetch_range()
behaviour.

Maybe it's worth having my first patch.
We don't lose the prefetch_range behavior.
Compiler should also be smart enough to manage the pref
inst optimizing the generated code.

Ok, I've taken the first patch for the .37 queue. I've given it a spin on
SH-2A and things seem to be ok there, too.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html