Re: [PATCH v2] LoongArch: Libvtv add loongarch support.

[Caroline Tice <cmtice@google.com>]

[-- Attachment #1: Type: text/plain, Size: 3460 bytes --]

I think that if VTV_PAGE_SIZE is not set to the actual size being used by
the system,  it could result in some unexpected failures.  I believe the
right thing to do in this case, since the size may vary, is to get the
actual size being used by the system and use that in the definition of
VTV_PAGE_SIZE.  So in include/vtv-permission.h you would have something
like:

+#elif defined(__loongarch_lp64)
+#define VTV_PAGE_SIZE sysconf(_SC_PAGE_SIZE)

Then you would have the accurate, correct size for the current system, and
there would be no need to update the
check in vtv_malloc.cc at all.

-- Caroline Tice
cmtice@google.com

On Tue, Sep 27, 2022 at 3:04 AM Lulu Cheng <chenglulu@loongson.cn> wrote:

>
> v1 - > v2:
>
> 1. When the macro __loongarch_lp64 is defined, the VTV_PAGE_SIZE is set to
> 64K.
> 2. In the vtv_malloc.cc file __vtv_malloc_init function, it does not check
>    whether VTV_PAGE_SIZE is equal to the system page size, if the macro
>    __loongarch_lp64 is defined.
>
> All regression tests of libvtv passed.
>
>                 === libvtv Summary ===
>
> # of expected passes            176
>
> But I haven't tested the performance yet.
>
> -----------------------------------
> Co-Authored-By: qijingwen <qijingwen@loongson.cn>
>
> include/ChangeLog:
>
>         * vtv-change-permission.h (defined):
>         (VTV_PAGE_SIZE): Under the loongarch64 architecture,
>         set VTV_PAGE_SIZE to 64K.
>
> libvtv/ChangeLog:
>
>         * configure.tgt: Add loongarch support.
>         * vtv_malloc.cc (defined): If macro __loongarch_lp64 is
>         defined, then don't check whether VTV_PAGE_SIZE is the
>         same as the system page size.
> ---
>  include/vtv-change-permission.h | 4 ++++
>  libvtv/configure.tgt            | 3 +++
>  libvtv/vtv_malloc.cc            | 5 +++++
>  3 files changed, 12 insertions(+)
>
> diff --git a/include/vtv-change-permission.h
> b/include/vtv-change-permission.h
> index 70bdad92bca..64e419c29d5 100644
> --- a/include/vtv-change-permission.h
> +++ b/include/vtv-change-permission.h
> @@ -48,6 +48,10 @@ extern void __VLTChangePermission (int);
>  #else
>  #if defined(__sun__) && defined(__svr4__) && defined(__sparc__)
>  #define VTV_PAGE_SIZE 8192
> +/* LoongArch architecture 64-bit system supports 4k,16k and 64k
> +   page size, which is set to the maximum value here.  */
> +#elif defined(__loongarch_lp64)
> +#define VTV_PAGE_SIZE 65536
>  #else
>  #define VTV_PAGE_SIZE 4096
>  #endif
> diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt
> index aa2a3f675b8..6cdd1e97ab1 100644
> --- a/libvtv/configure.tgt
> +++ b/libvtv/configure.tgt
> @@ -50,6 +50,9 @@ case "${target}" in
>         ;;
>    x86_64-*-darwin[1]* | i?86-*-darwin[1]*)
>         ;;
> +  loongarch*-*-linux*)
> +       VTV_SUPPORTED=yes
> +       ;;
>    *)
>         ;;
>  esac
> diff --git a/libvtv/vtv_malloc.cc b/libvtv/vtv_malloc.cc
> index 67c5de6d4e9..45804b8d7f8 100644
> --- a/libvtv/vtv_malloc.cc
> +++ b/libvtv/vtv_malloc.cc
> @@ -212,6 +212,11 @@ __vtv_malloc_init (void)
>
>  #if defined (__CYGWIN__) || defined (__MINGW32__)
>    if (VTV_PAGE_SIZE != sysconf_SC_PAGE_SIZE())
> +#elif defined (__loongarch_lp64)
> +  /* I think that under the LoongArch 64-bit system, VTV_PAGE_SIZE is set
> +     to the maximum value of 64K supported by the system, so there is no
> +     need to judge here.  */
> +  if (false)
>  #else
>    if (VTV_PAGE_SIZE != sysconf (_SC_PAGE_SIZE))
>  #endif
> --
> 2.31.1
>
>