Re: [ros-dev] [ros-diffs] [sginsberg] 69248: [HAL] Get rid off REGISTERCALL ("regparm(3)" for GCC) and replace with good old NTAPI (stdcall). Such an obsession with avoiding stack usage is not healthy nor makes much sense t...

[Alex Ionescu]

You seem to have a huge misunderstanding of modern processor architecture.
Everything you said is false. In fact, this is why x64 uses register
passing, as does ARM and every other major architecture except x86.
Push/pop operations are extremely expensive and break pipelining. The
compiler will then place the parameters in registers anyways after the fact.

Suffice it to say though, the original reason these were regparm/fastcall
is to easily allow them to be called from assembly at the time.

Best regards,
Alex Ionescu

On Tue, Sep 15, 2015 at 7:03 PM, [e-mail] wrote:
> Author: sginsberg
> Date: Tue Sep 15 23:03:42 2015
> New Revision: 69248
>
> URL: http://svn.reactos.org/svn/reactos?rev=69248&view=rev
> Log:
> [HAL] Get rid off REGISTERCALL ("regparm(3)" for GCC) and replace with
> good old NTAPI (stdcall). Such an obsession with avoiding stack usage is
> not healthy nor makes much sense today (or even a long time before today)
> with processors that have a decent L1 cache, whose "cost of access" is
> basically the same as to that of a register, and with processors being
> capable of recognising basic access patterns to ensure frequently used
> memory (read: stack) is in the cache. Slapping FASTCALL/regparm on
> frequently used code does not ensure it actually operates faster. You want
> to know what really hurts performance (and cache)? Slapping FORCEINLINE on
> everything like if it was some kind of asm macro, making code needlessly
> bloated.
>
> Modified:
>     trunk/reactos/hal/halx86/include/halp.h
>     trunk/reactos/hal/halx86/up/pic.c
>
> Modified: trunk/reactos/hal/halx86/include/halp.h
> URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/include/halp.h?r...
>
> ==============================================================================
> --- trunk/reactos/hal/halx86/include/halp.h     [iso-8859-1] (original)
> +++ trunk/reactos/hal/halx86/include/halp.h     [iso-8859-1] Tue Sep 15
> 23:03:42 2015
> @@ -10,12 +10,6 @@
>  #define INIT_SECTION /* Done via alloc_text for MSC */
>  #endif
>
> -#ifdef _MSC_VER
> -#define REGISTERCALL FASTCALL
> -#else
> -#define REGISTERCALL __attribute__((regparm(3)))
> -#endif
>
> #ifdef CONFIG_SMP
>  #define HAL_BUILD_TYPE (DBG ? PRCB_BUILD_DEBUG : 0)
> @@ -430,14 +424,14 @@
>
> typedef
>  BOOLEAN
> -( REGISTERCALL *PHAL_DISMISS_INTERRUPT)(
> +(NTAPI *PHAL_DISMISS_INTERRUPT)(
>      IN KIRQL Irql,
>      IN ULONG Irq,
>      OUT PKIRQL OldIrql
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrqGeneric(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -445,7 +439,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq15(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -453,7 +447,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq13(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -461,7 +455,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq07(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -469,7 +463,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrqLevel(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -477,7 +471,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq15Level(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -485,7 +479,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq13Level(
>      IN KIRQL Irql,
>      IN ULONG Irq,
> @@ -493,7 +487,7 @@
>  );
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq07Level(
>      IN KIRQL Irql,
>      IN ULONG Irq,
>
> Modified: trunk/reactos/hal/halx86/up/pic.c
> URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/hal/halx86/up/pic.c?rev=692...
>
> ==============================================================================
> --- trunk/reactos/hal/halx86/up/pic.c   [iso-8859-1] (original)
> +++ trunk/reactos/hal/halx86/up/pic.c   [iso-8859-1] Tue Sep 15 23:03:42
> 2015
> @@ -843,7 +843,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrqGeneric(IN KIRQL Irql,
>                        IN ULONG Irq,
>                        OUT PKIRQL OldIrql)
> @@ -853,7 +853,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq15(IN KIRQL Irql,
>                   IN ULONG Irq,
>                   OUT PKIRQL OldIrql)
> @@ -889,7 +889,7 @@
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq13(IN KIRQL Irql,
>                   IN ULONG Irq,
>                   OUT PKIRQL OldIrql)
> @@ -902,7 +902,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq07(IN KIRQL Irql,
>                   IN ULONG Irq,
>                   OUT PKIRQL OldIrql)
> @@ -986,7 +986,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrqLevel(IN KIRQL Irql,
>                      IN ULONG Irq,
>                      OUT PKIRQL OldIrql)
> @@ -996,7 +996,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq15Level(IN KIRQL Irql,
>                        IN ULONG Irq,
>                        OUT PKIRQL OldIrql)
> @@ -1031,7 +1031,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq13Level(IN KIRQL Irql,
>                        IN ULONG Irq,
>                        OUT PKIRQL OldIrql)
> @@ -1044,7 +1044,7 @@
>  }
>
> BOOLEAN
> -REGISTERCALL
> +NTAPI
>  HalpDismissIrq07Level(IN KIRQL Irql,
>                        IN ULONG Irq,
>                        OUT PKIRQL OldIrql)