Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Port to LoongArch64 #257

Closed
wants to merge 2 commits into from
Closed

Port to LoongArch64 #257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c70ade9
Port to LoongArch64
znley Aug 23, 2021
95422ab
fix: fp cannot be used in 'asm' on loongarch64
May 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#
# Adding the -Os flag fixes the problem.
or (is_linux and plat_machine == "riscv64")
or (is_linux and plat_machine == "loongarch64")
):
global_compile_args.append("-Os")

Expand Down
31 changes: 31 additions & 0 deletions src/greenlet/platform/switch_loongarch64_linux.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#define STACK_REFPLUS 1

#ifdef SLP_EVAL
#define STACK_MAGIC 0

#define REGS_TO_SAVE "s0", "s1", "s2", "s3", "s4", "s5", \
"s6", "s7", "s8", "fp", \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure fp can be included? I see this in my independent port before I saw your PR:

loongarch64-unknown-linux-gnu-gcc -fPIC -I/home/xenon/greenlet-venv/include -I/usr/include/python3.9 -c src/greenlet/greenlet.c -o build/temp.linux-loongarch64-3.9/src/greenlet/greenlet.o
In file included from src/greenlet/slp_platformselect.h:59,
                 from src/greenlet/greenlet.c:370:
src/greenlet/platform/switch_loongarch64_unix.h: In function ‘slp_switch’:
src/greenlet/platform/switch_loongarch64_unix.h:31:1: error: $r22 cannot be used in ‘asm’ here
   31 | }
      | ^
error: command '/usr/bin/loongarch64-unknown-linux-gnu-gcc' failed with exit code 1

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am sure fp can be included. I think your error message is caused by gcc compilation optimization. This is my compilation flags.

loongarch64-loongson-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -O2 -g -D_GNU_SOURCE -fPIC -fwrapv -O2 -g -D_GNU_SOURCE -fPIC -fwrapv -O2 -g -D_GNU_SOURCE -fPIC -fwrapv -fPIC -I/usr/include/python3.6m -c src/greenlet/greenlet.c -o build/temp.linux-loongarch64-3.6/src/greenlet/greenlet.o

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's not good for portability, you can't force one particular set of CFLAGS, due to different distros just doing their job differently. I know that fp/s9 actually should be saved, but how do we force it in this case? I'm using the latest upstream submission for my toolchain and kernel, and a self-compiled Gentoo userland; let me know if you need the Gentoo tarball to reproduce the issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can reproduce this issue in other way. This is indeed a problem.

Inspired by your question, I found this the setup.py. I think we should add appropriate compilation optimization flags for loongarch64 here.

if ((sys.platform == "openbsd4" and os.uname()[-1] == "i386")
    or ("-with-redhat-3." in platform.platform() and platform.machine() == 'i686')
    or (sys.platform == "sunos5" and os.uname()[-1] == "sun4v")
    or ("SunOS" in platform.platform() and platform.machine() == "sun4v")
    or (sys.platform == "linux" and platform.machine() == "ppc")):
    os.environ["CFLAGS"] = ("%s %s" % (os.environ.get("CFLAGS", ""), "-Os")).lstrip()

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optimization flags are not the proper way to solve this, in my opinion, because they tend to depend on intricate compiler internals to work. The "solution" will likely be fragile and break when the stars don't align.

Actually, the way most other projects manually save/restore preserved registers is to write concrete asm fragments, even whole .S files; doing this via clobber list is something I have never seen before, yet in this project every port does that. How about noticing your compiler team about this so they can provide some input? @-mentioning them here should be enough.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@merore 所以这里正确的处理方式是什么？python setup.py bdist_wheel是一样的报上面的错

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Duanexiao Please refer to the latest commit.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Duanexiao Please refer to the latest commit.

gevent项目我看贵司还没提兼容loongarch64的代码？但是龙芯的pypi里面有这个

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gevent项目我看贵司还没提兼容loongarch64的代码？但是龙芯的pypi里面有这个

code here https://github.com/merore/greenlet/commits/master

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gevent项目我看贵司还没提兼容loongarch64的代码？但是龙芯的pypi里面有这个

code here https://github.com/merore/greenlet/commits/master

我说的是gevent项目

"f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31"

static int
slp_switch(void)
{
register int ret;
register long *stackref, stsizediff;
__asm__ volatile ("" : : : REGS_TO_SAVE);
__asm__ volatile ("move %0, $sp" : "=r" (stackref) : );
{
SLP_SAVE_STATE(stackref, stsizediff);
__asm__ volatile (
"add.d $sp, $sp, %0\n\t"
: /* no outputs */
: "r" (stsizediff)
);
SLP_RESTORE_STATE();
}
__asm__ volatile ("" : : : REGS_TO_SAVE);
__asm__ volatile ("move %0, $zero" : "=r" (ret) : );
return ret;
}

#endif
2 changes: 2 additions & 0 deletions src/greenlet/slp_platformselect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ extern "C" {
#include "platform/switch_s390_unix.h" /* Linux/S390 */
#elif defined(__GNUC__) && defined(__s390x__) && defined(__linux__)
#include "platform/switch_s390_unix.h" /* Linux/S390 zSeries (64-bit) */
#elif defined(__GNUC__) && defined(__loongarch64) && defined(__linux__)
#include "platform/switch_loongarch64_linux.h" /* Linux/LoongArch64 */
#elif defined(__GNUC__) && defined(__arm__)
#ifdef __APPLE__
#include <TargetConditionals.h>
Expand Down