diff mbox series

MIPS: Take in account load hazards for HI/LO restoring

Message ID 20231012162027.3411684-1-lis8215@gmail.com (mailing list archive)
State Rejected
Headers show
Series MIPS: Take in account load hazards for HI/LO restoring | expand

Commit Message

Siarhei Volkau Oct. 12, 2023, 4:20 p.m. UTC
MIPS CPUs usually have 1 to 4 cycles load hazards, thus doing load
and right after move to HI/LO will usually stall the pipeline for
significant amount of time. Let's take it into account and separate
loads and mthi/lo in instruction sequence.

The patch uses t6 and t7 registers as temporaries in addition to t8.

The patch tries to deal with SmartMIPS, but I know little about and
haven't tested it.

Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
---
 arch/mips/include/asm/stackframe.h | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Siarhei Volkau April 29, 2024, 6:42 a.m. UTC | #1
Ping. The patch looks abandoned.

Paul, could you recommend right persons / lists for that ?

чт, 12 окт. 2023 г. в 19:21, Siarhei Volkau <lis8215@gmail.com>:
>
> MIPS CPUs usually have 1 to 4 cycles load hazards, thus doing load
> and right after move to HI/LO will usually stall the pipeline for
> significant amount of time. Let's take it into account and separate
> loads and mthi/lo in instruction sequence.
>
> The patch uses t6 and t7 registers as temporaries in addition to t8.
>
> The patch tries to deal with SmartMIPS, but I know little about and
> haven't tested it.
>
> Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> ---
>  arch/mips/include/asm/stackframe.h | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
> index a8705aef47e1..3821d91b00fd 100644
> --- a/arch/mips/include/asm/stackframe.h
> +++ b/arch/mips/include/asm/stackframe.h
> @@ -308,17 +308,11 @@
>                 jal     octeon_mult_restore
>  #endif
>  #ifdef CONFIG_CPU_HAS_SMARTMIPS
> -               LONG_L  $24, PT_ACX(sp)
> -               mtlhx   $24
> -               LONG_L  $24, PT_HI(sp)
> -               mtlhx   $24
> -               LONG_L  $24, PT_LO(sp)
> -               mtlhx   $24
> -#elif !defined(CONFIG_CPU_MIPSR6)
> +               LONG_L  $14, PT_ACX(sp)
> +#endif
> +#if defined(CONFIG_CPU_HAS_SMARTMIPS) || !defined(CONFIG_CPU_MIPSR6)
>                 LONG_L  $24, PT_LO(sp)
> -               mtlo    $24
> -               LONG_L  $24, PT_HI(sp)
> -               mthi    $24
> +               LONG_L  $15, PT_HI(sp)
>  #endif
>  #ifdef CONFIG_32BIT
>                 cfi_ld  $8, PT_R8, \docfi
> @@ -327,6 +321,14 @@
>                 cfi_ld  $10, PT_R10, \docfi
>                 cfi_ld  $11, PT_R11, \docfi
>                 cfi_ld  $12, PT_R12, \docfi
> +#ifdef CONFIG_CPU_HAS_SMARTMIPS
> +               mtlhx   $14
> +               mtlhx   $15
> +               mtlhx   $24
> +#elif !defined(CONFIG_CPU_MIPSR6)
> +               mtlo    $24
> +               mthi    $15
> +#endif
>                 cfi_ld  $13, PT_R13, \docfi
>                 cfi_ld  $14, PT_R14, \docfi
>                 cfi_ld  $15, PT_R15, \docfi
> --
> 2.41.0
>
Thomas Bogendoerfer April 29, 2024, 8:18 a.m. UTC | #2
On Thu, Oct 12, 2023 at 07:20:27PM +0300, Siarhei Volkau wrote:
> MIPS CPUs usually have 1 to 4 cycles load hazards, thus doing load
> and right after move to HI/LO will usually stall the pipeline for
> significant amount of time. Let's take it into account and separate
> loads and mthi/lo in instruction sequence.
> 
> The patch uses t6 and t7 registers as temporaries in addition to t8.
> 
> The patch tries to deal with SmartMIPS, but I know little about and
> haven't tested it.
> 
> Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> ---
>  arch/mips/include/asm/stackframe.h | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
> index a8705aef47e1..3821d91b00fd 100644
> --- a/arch/mips/include/asm/stackframe.h
> +++ b/arch/mips/include/asm/stackframe.h
> @@ -308,17 +308,11 @@
>  		jal	octeon_mult_restore
>  #endif
>  #ifdef CONFIG_CPU_HAS_SMARTMIPS
> -		LONG_L	$24, PT_ACX(sp)
> -		mtlhx	$24
> -		LONG_L	$24, PT_HI(sp)
> -		mtlhx	$24
> -		LONG_L	$24, PT_LO(sp)
> -		mtlhx	$24
> -#elif !defined(CONFIG_CPU_MIPSR6)
> +		LONG_L	$14, PT_ACX(sp)
> +#endif
> +#if defined(CONFIG_CPU_HAS_SMARTMIPS) || !defined(CONFIG_CPU_MIPSR6)

isn't that just #ifndef CONFIG_CPU_MIPSR6 ? 

Thomas.
Thomas Bogendoerfer April 29, 2024, 8:30 a.m. UTC | #3
On Mon, Apr 29, 2024 at 10:18:57AM +0200, Thomas Bogendoerfer wrote:
> On Thu, Oct 12, 2023 at 07:20:27PM +0300, Siarhei Volkau wrote:
> > MIPS CPUs usually have 1 to 4 cycles load hazards, thus doing load
> > and right after move to HI/LO will usually stall the pipeline for
> > significant amount of time. Let's take it into account and separate
> > loads and mthi/lo in instruction sequence.
> > 
> > The patch uses t6 and t7 registers as temporaries in addition to t8.
> > 
> > The patch tries to deal with SmartMIPS, but I know little about and
> > haven't tested it.
> > 
> > Signed-off-by: Siarhei Volkau <lis8215@gmail.com>
> > ---
> >  arch/mips/include/asm/stackframe.h | 22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
> > index a8705aef47e1..3821d91b00fd 100644
> > --- a/arch/mips/include/asm/stackframe.h
> > +++ b/arch/mips/include/asm/stackframe.h
> > @@ -308,17 +308,11 @@
> >  		jal	octeon_mult_restore
> >  #endif
> >  #ifdef CONFIG_CPU_HAS_SMARTMIPS
> > -		LONG_L	$24, PT_ACX(sp)
> > -		mtlhx	$24
> > -		LONG_L	$24, PT_HI(sp)
> > -		mtlhx	$24
> > -		LONG_L	$24, PT_LO(sp)
> > -		mtlhx	$24
> > -#elif !defined(CONFIG_CPU_MIPSR6)
> > +		LONG_L	$14, PT_ACX(sp)
> > +#endif
> > +#if defined(CONFIG_CPU_HAS_SMARTMIPS) || !defined(CONFIG_CPU_MIPSR6)
> 
> isn't that just #ifndef CONFIG_CPU_MIPSR6 ? 

and if yes, I prefer to have the same structure as for the move to
registers later like

#ifdef CONFIG_CPU_HAS_SMARTMIPS
.. do the SMARTMIPS things
elif !defined(CONFIG_CPU_MIPSR6)
.. do normal hi/lo
#endif

that way it's more clear whats happening depending on selected
options.

Thomas.
diff mbox series

Patch

diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
index a8705aef47e1..3821d91b00fd 100644
--- a/arch/mips/include/asm/stackframe.h
+++ b/arch/mips/include/asm/stackframe.h
@@ -308,17 +308,11 @@ 
 		jal	octeon_mult_restore
 #endif
 #ifdef CONFIG_CPU_HAS_SMARTMIPS
-		LONG_L	$24, PT_ACX(sp)
-		mtlhx	$24
-		LONG_L	$24, PT_HI(sp)
-		mtlhx	$24
-		LONG_L	$24, PT_LO(sp)
-		mtlhx	$24
-#elif !defined(CONFIG_CPU_MIPSR6)
+		LONG_L	$14, PT_ACX(sp)
+#endif
+#if defined(CONFIG_CPU_HAS_SMARTMIPS) || !defined(CONFIG_CPU_MIPSR6)
 		LONG_L	$24, PT_LO(sp)
-		mtlo	$24
-		LONG_L	$24, PT_HI(sp)
-		mthi	$24
+		LONG_L	$15, PT_HI(sp)
 #endif
 #ifdef CONFIG_32BIT
 		cfi_ld	$8, PT_R8, \docfi
@@ -327,6 +321,14 @@ 
 		cfi_ld	$10, PT_R10, \docfi
 		cfi_ld	$11, PT_R11, \docfi
 		cfi_ld	$12, PT_R12, \docfi
+#ifdef CONFIG_CPU_HAS_SMARTMIPS
+		mtlhx	$14
+		mtlhx	$15
+		mtlhx	$24
+#elif !defined(CONFIG_CPU_MIPSR6)
+		mtlo	$24
+		mthi	$15
+#endif
 		cfi_ld	$13, PT_R13, \docfi
 		cfi_ld	$14, PT_R14, \docfi
 		cfi_ld	$15, PT_R15, \docfi