[PATCH v2] LoongArch: Libvtv add loongarch support.

[PATCH v2] LoongArch: Libvtv add loongarch support.

[Lulu Cheng]

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

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

[Xi Ruoyao]

On Tue, 2022-09-27 at 15:49 +0800, Lulu Cheng wrote:
>  #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)

I think "if (false)" can trigger some compiler warnings...

Still not sure if the maximum value is always correct (+ Caroline for a
confirmation).  If it's correct I'd suggest...

>  #else
>    if (VTV_PAGE_SIZE != sysconf (_SC_PAGE_SIZE))

if (VTV_PAGE_SIZE % sysconf (_SC_PAGE_SIZE) != 0)

>  #endif

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

[Lulu Cheng]

在 2022/9/27 下午7:44, Xi Ruoyao 写道:
> On Tue, 2022-09-27 at 15:49 +0800, Lulu Cheng wrote:
>>   #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)
> I think "if (false)" can trigger some compiler warnings...
>
> Still not sure if the maximum value is always correct (+ Caroline for a
> confirmation).  If it's correct I'd suggest...
>
>>   #else
>>     if (VTV_PAGE_SIZE != sysconf (_SC_PAGE_SIZE))
> if (VTV_PAGE_SIZE % sysconf (_SC_PAGE_SIZE) != 0)


If the setting to the maximum value is correct,

I also think it is better to achieve this.

>
>>   #endif

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

[Caroline Tice]

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

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))
>

Is "if (VTV_PAGE_SIZE != sysconf (_SC_PAGE_SIZE))" going to fail for
loongarch?  If not, why do you need to insert anything here at all?  If so,
perhaps you could write something similar to sysconf_SC_PAGE_SIZE for
loongarch (as was done for __CYGWIN__ & __MINGW32__)?

-- Caroline
cmtice@google.com



>  #endif
> --
> 2.31.1
>
>

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

[Xi Ruoyao]

On Mon, 2022-10-10 at 10:49 -0700, Caroline Tice via Gcc-patches wrote:
> Is "if (VTV_PAGE_SIZE != sysconf (_SC_PAGE_SIZE))" going to fail for
> loongarch?

Because LoongArch systems may have 4KB, 16KB, or 64KB pages.

> If not, why do you need to insert anything here at all?  If so,
> perhaps you could write something similar to sysconf_SC_PAGE_SIZE for
> loongarch (as was done for __CYGWIN__ & __MINGW32__)?

I'd like to ask a question: if we set VTV_PAGE_SIZE to 64KB and make the
special case, will libvtv work for 4KB and 16KB pages?  (If I read code
correctly, setting VTV_PAGE_SIZE to 4KB will obviously break 16KB or
64KB configuration.)

If VTV_PAGE_SIZE == sysconf (_SC_PAGE_SIZE) is strictly required for
libvtv we'll have to keep the check as-is and then we'll only support
16KB page configuration (which is the default in Linux kernel
configuration for LoongArch).

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

[Caroline Tice]

[-- 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
>
>

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

[Lulu Cheng]

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


在 2022/10/12 上午4:57, Caroline Tice 写道:
> 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.


     /* Page-aligned symbol to mark beginning of .vtable_map_vars 
section.  */
     char _vtable_map_vars_start []
     __attribute__ ((__visibility__ ("protected"), used, 
aligned(VTV_PAGE_SIZE),
             section(".vtable_map_vars")))
       = { };

The above code is in the libgcc/vtv_start.c file. Alignment (aligned 
(alignment) ) must be an
integer constant power of 2. So setting VTV_PAGE_SIZE as a variable is 
not advisable.


As xiruoyao notes, the default value for the LoongArch Linux kernel 
configuration is 16KB.

So let's set VTV_PAGE_SIZE to 16KB first and I will indicate in the 
submission information

that only 16KB pages are supported.

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

[Caroline Tice]

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

On Tue, Oct 11, 2022 at 7:52 PM Lulu Cheng <chenglulu@loongson.cn> wrote:

>
> 在 2022/10/12 上午4:57, Caroline Tice 写道:
>
> 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.
>
>
>     /* Page-aligned symbol to mark beginning of .vtable_map_vars section.
> */
>     char _vtable_map_vars_start []
>     __attribute__ ((__visibility__ ("protected"), used,
> aligned(VTV_PAGE_SIZE),
>             section(".vtable_map_vars")))
>       = { };
>
> The above code is in the libgcc/vtv_start.c file. Alignment (aligned
> (alignment) ) must be an
> integer constant power of 2. So setting VTV_PAGE_SIZE as a variable is not
> advisable.
>
>
> As xiruoyao notes, the default value for the LoongArch Linux kernel
> configuration is 16KB.
>
> So let's set VTV_PAGE_SIZE to 16KB first and I will indicate in the
> submission information
>
> that only 16KB pages are supported.
>
>
> OK, that would work if you want to go that way.