[v2] LoongArch: Libvtv add loongarch support.
Checks
Commit Message
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(+)
Comments
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
在 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
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
>
>
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).
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
>
>
在 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.
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.
@@ -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
@@ -50,6 +50,9 @@ case "${target}" in
;;
x86_64-*-darwin[1]* | i?86-*-darwin[1]*)
;;
+ loongarch*-*-linux*)
+ VTV_SUPPORTED=yes
+ ;;
*)
;;
esac
@@ -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