Skip to content

Add loongarch64 support#4914

Merged
youknowone merged 6 commits intoRustPython:mainfrom
xiangzhai:loongarch64
Apr 23, 2023
Merged

Add loongarch64 support#4914
youknowone merged 6 commits intoRustPython:mainfrom
xiangzhai:loongarch64

Conversation

@xiangzhai
Copy link
Contributor

@xiangzhai xiangzhai commented Apr 21, 2023

Hi,

cargo build && test, all tests pass:

     Running unittests src/lib.rs (target/debug/deps/rustpython-2c061bc00c2f70a2)

running 1 test
Hello
Hello
test tests::test_run_script ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.38s

     Running unittests src/main.rs (target/debug/deps/rustpython-f9242e878d797bdd)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests rustpython

running 1 test
test src/lib.rs - (line 8) - compile ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.15s

Please review my patch.

Thanks,
Leslie Zhai

youknowone
youknowone requested changes Apr 21, 2023
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Related CPython issues
ref

Lib/test/test_sysconfig.py Outdated
machine = platform.machine()
suffix = sysconfig.get_config_var('EXT_SUFFIX')
if re.match('(aarch64|arm|mips|ppc|powerpc|s390|sparc)', machine):
if re.match('(aarch64|arm|loongarch64|mips|ppc|powerpc|s390|sparc)', machine):
Copy link
Member

@youknowone youknowone Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We generally don't edit this file ourselves because it mostly came from CPython.
But I understand this is necessary to correctly support the architecture.
Not to lose the changes when we update this file to new version,
Adding a comment is useful.

Suggested change
if re.match('(aarch64|arm|loongarch64|mips|ppc|powerpc|s390|sparc)', machine):
# XXX: RUSTPYTHON; loongarch64 is locally added in github #4914
if re.match('(aarch64|arm|loongarch64|mips|ppc|powerpc|s390|sparc)', machine):

And also, I found the related CPython patch doesn't include this change. Is it necessary in RustPython?

Copy link
Contributor Author

@xiangzhai xiangzhai Apr 23, 2023

Choose a reason for hiding this comment

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

Thanks for your careful review!

It is not necessary in RustPython.

Fixed.

youknowone
youknowone approved these changes Apr 23, 2023
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

Thank you!

@xiangzhai xiangzhai requested a review from youknowone April 23, 2023 03:19
@xiangzhai
Copy link
Contributor Author

xiangzhai commented Apr 23, 2023

Hi @youknowone

I fixed cargo fmt and clippy issues and ran the test for Ubuntu 22.04 x86_64:

cargo fmt --all -- --check
cargo clippy --no-default-features --features stdlib,zlib,importlib,encodings,ssl,jit -p rustpython-common -p rustpython-compiler-core -p rustpython-compiler -p rustpython-codegen -p rustpython-parser -p rustpython-vm -p rustpython-stdlib -p rustpython-jit -p rustpython-derive -p rustpython -- -Dwarnings

Please review it again!

Thanks,
Leslie Zhai

youknowone
youknowone reviewed Apr 23, 2023
View reviewed changes
stdlib/src/mmap.rs Outdated
MADV_MERGEABLE, MADV_NOHUGEPAGE, MADV_REMOVE, MADV_UNMERGEABLE,
};

#[allow(unused_imports)]
Copy link
Member

@youknowone youknowone Apr 23, 2023

Choose a reason for hiding this comment

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

This is supposed to be used by #[pyattr]

Suggested change
#[allow(unused_imports)]
#[pyattr]

Because we usually use cfg - pyattr order, placing it under cfg will be better.

Copy link
Contributor Author

@xiangzhai xiangzhai Apr 23, 2023

Choose a reason for hiding this comment

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

Fixed

youknowone
youknowone reviewed Apr 23, 2023
View reviewed changes
stdlib/src/mmap.rs Outdated
)
)
))]
#[allow(unused_imports)]
Copy link
Member

@youknowone youknowone Apr 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
#[allow(unused_imports)]
#[pyattr]

The value has to be used by pyattr to be exposed to python side

Copy link
Contributor Author

@xiangzhai xiangzhai Apr 23, 2023

Choose a reason for hiding this comment

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

I got :)

@xiangzhai xiangzhai requested a review from youknowone April 23, 2023 03:48
@youknowone youknowone merged commit 2c90b12 into RustPython:main Apr 23, 2023
@youknowone
Copy link
Member

youknowone commented Apr 23, 2023

Thank you for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone Awaiting requested review from youknowone

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@xiangzhai @youknowone