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.

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

fix: build on risc-v #9215

Closed
wants to merge 3 commits into from
Closed

fix: build on risc-v #9215

wants to merge 3 commits into from

Conversation

XieJiSS
Copy link
Contributor

@XieJiSS XieJiSS commented Nov 25, 2021

Patch is modified from https://reviews.llvm.org/file/data/du5ol5zctyqw53ma7dwz/PHID-FILE-knherxziu4tl4erti5ab/file

Tested on Arch Linux riscv64gc (qemu)

UPDATE: Seems like the above link is broken, so I tried to search for a link pointing to the original merge request. It turned out to me that the LLVM guys are cherry-picking from google/benchmark, and the upstream should be this:

https://github.com/google/benchmark/blob/808571a52fd6cc7e9f0788e08f71f0f4175b6673/src/cycleclock.h#L190

@facebook-github-bot
Copy link
Contributor

Hi @XieJiSS!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@adamretter
Copy link
Collaborator

@XieJiSS have you considered also sending this fix upstream to the Toku code base? It might help us in future if we ever want to pull in changes from there too

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Nov 25, 2021

@adamretter Sure, I'll do so ASAP.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Nov 25, 2021

@adamretter I've filed a new PR regarding this to upstream.

percona/PerconaFT#461 (toku_time is part of the TokuDB which is now PerconaFT)

@adamretter
Copy link
Collaborator

@XieJiSS I would like to give this a test, I can get an emulated RiscV system up under QEMU. I am not sure of the best operations to test this. I am afraid my knowledge of assembly coding is very cursory. Do you have suggestions for how I might test this?

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Nov 27, 2021

@adamretter If by stating "test" you mean to test the assembly codes, how about creating a new .c file and paste toku_time.h#L125..L160 into it? AFAIK gcc is now usable on risc-v platforms.

  1. Download the archriscv package, unzip. (see the wget shell command below)
  2. create the test.c and edit it: vim ./test.c (see https://gist.github.com/XieJiSS/18acd12685ecc0913d43d6bcfb3baefd)
  3. sudo vim path/to/archriscv/root/test.c
  4. systemd-nspawn
  5. compile & run on both platforms, check output
# append these 2 lines to /etc/pacman.conf
[archlinuxcn]
Server = https://repo.archlinuxcn.org/$arch
# This is an Arch Linux x86_64 server

# These two packages are in [archlinuxcn]
sudo pacman -S qemu-user-static binfmt-qemu-static

# An rv64gc image. If root password is needed in this image, it should be: sifive
wget -c https://archriscv.felixc.at/images/archriscv-20210601.tar.zst
sha512sum archriscv-20210601.tar.zst
# 6f012a169fe6f1ea15aeb3283091466e7992f78d823951ee2170940fa030e7fa2394aee11bf67c29943d21579ab42d2262a3d5ca973b5de8be779f338ba1dd44  archriscv-20210601.tar.zst
mkdir ~/archriscv
sudo bsdtar -xvf archriscv-20210601.tar.zst -C archriscv

vim ./test.c  # edit file
sudo cp ./test.c ~/archriscv/root/

gcc ./test.c
./a.out

Then, enter the archriscv container:

sudo systemd-nspawn -D ~/archriscv/ --machine archriscv -a -U

gcc ./test.c
./a.out

My test result is:

17433587739862234
17433592096901640

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 17, 2021

Hi @adamretter , is there anything I can help regarding the test?

@adamretter
Copy link
Collaborator

I've had trouble running Ubuntu riscv qemu. Seems there are some bugs in the vmbuilder scripts, I will dig out the details and paste here later

@adamretter
Copy link
Collaborator

@XieJiSS So I am using a Ubuntu 20.4 system to try and install a RISC-V Qemu VM for testing your code. However, I have been unable to get the RISC-V VM successfully installed.

I am basically following these instructions:

  1. Install QEMU Support for RISC-V on the host
$ apt-get install qemu-system-misc opensbi u-boot-qemu qemu-utils
  1. Get the Ubuntu 20.04 LTS (focal) image for SiFive Umatched RISC-V
$ wget https://cdimage.ubuntu.com/releases/20.04.3/release/ubuntu-20.04.3-preinstalled-server-riscv64+unmatched.img.xz
$ xz -dk ubuntu-20.04.3-preinstalled-server-riscv64+unmatched.img.xz
  1. Create a RISC-V VM
$ apt-get install virtinst
$ virt-install --debug --name riscv --memory 4096 --arch riscv64 --machine sifive_u --vcpus 4 --os-variant  ubuntu20.04 --boot loader=/usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf,kernel=/usr/lib/u-boot/qemu-riscv64_smode/uboot.elf --import --disk /tmp/ubuntu-20.04.3-preinstalled-server-riscv64+unmatched.img,format=raw,bus=virtio

However, I cannot start the VM, and end up with what I think is the same bug as this: https://www.spinics.net/lists/virt-tools/msg14419.html

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 18, 2021

Hi @adamretter , looks like currently the libvirt bug is difficult to deal with.

Here at PLCTLab, we provides an open CI infrastructure for RISC-V porting ( https://ci.rvperf.org ). Would it be possible for you to test facebook/rocksdb on the CI? We already have nodejs RISC-V CI hosted on this CI (ref nodejs/node#37856).

Another choice is that we can provide SSH access to SiFive unmatched boards, so that you can test the code on (physical) RISC-V boards.

I'll also have a look on how to get RISC-V qemu toolchain work successfully on Ubuntu.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 18, 2021

And I can't figure out why the code format CI test is failing, make format told me that Nothing needs to be reformatted

@adamretter
Copy link
Collaborator

@XieJiSS I could add RocksDB to your CI as an experimental target, but how do I sign-up for that.
Also if you can provide access to a RISC-V vm, then I could use that, and even produce some experimental release builds of RocksJava for RISC-V if that is of interest?

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 19, 2021

Hi @adamretter , we are happy to see RocksDB being added to rvperf CI. I can create a job by opening a PR at https://github.com/plctlab/riscv-ci (which, when merged, will automatically add a job to the CI), but I may need help with creating a working build script (cross-build might be needed) on ubuntu 20.04 (the CI host os). I'll discuss this with my mentor asap.

As for the SSH access to SiFive unmatched boards, I'll add an account for you in (I hope) a week or so. It depends on the efficiency of the internal workflow at PLCT. Is it ok to use your pub keys retrieved from https://github.com/adamretter.keys ?

@adamretter
Copy link
Collaborator

@XieJiSS Yes the 2nd SSH key will work fine thanks.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Dec 26, 2021

@adamretter Hi, I've sent you an email with ssh access related information.

@adamretter
Copy link
Collaborator

adamretter commented Jan 7, 2022

@XieJiSS Thanks for access to the RISCV system, I have managed to test your code there and it appears to work.

Where above you say:

I've filed a new PR regarding this to upstream.

Would you mind inserting a link to that PR in that message, so we have a record of it?

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Jan 23, 2022

@adamretter Sure!

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Feb 10, 2022

@adamretter Hi Adam, is there anything else I need to do to get this PR merged? Thanks!

@facebook-github-bot
Copy link
Contributor

@mrambacher has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@XieJiSS has updated the pull request. You must reimport the pull request before landing.

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Feb 16, 2022

Oops... I've just noticed that the clang format CI check is failing, so I re-formatted the code as the linter advised. Hope I didn't break anything

@facebook-github-bot
Copy link
Contributor

@mrambacher has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Mar 1, 2022
Summary:
Related to: #9215

* Adds build_detect_platform support for RISCV on Linux (at least on SiFive Unmatched platforms)

This still leaves some linking issues on RISCV remaining (e.g. when building `db_test`):
```
/usr/bin/ld: ./librocksdb_debug.a(memtable.o): in function `__gnu_cxx::new_allocator<char>::deallocate(char*, unsigned long)':
/usr/include/c++/10/ext/new_allocator.h:133: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(memtable.o): in function `std::__atomic_base<bool>::compare_exchange_weak(bool&, bool, std::memory_order, std::memory_order)':
/usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: /usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: /usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: /usr/include/c++/10/bits/atomic_base.h:464: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(memtable.o):/usr/include/c++/10/bits/atomic_base.h:464: more undefined references to `__atomic_compare_exchange_1' follow
/usr/bin/ld: ./librocksdb_debug.a(db_impl.o): in function `rocksdb::DBImpl::NewIteratorImpl(rocksdb::ReadOptions const&, rocksdb::ColumnFamilyData*, unsigned long, rocksdb::ReadCallback*, bool, bool)':
/home/adamretter/rocksdb/db/db_impl/db_impl.cc:3019: undefined reference to `__atomic_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(write_thread.o): in function `rocksdb::WriteThread::Writer::CreateMutex()':
/home/adamretter/rocksdb/./db/write_thread.h:205: undefined reference to `__atomic_compare_exchange_1'
/usr/bin/ld: ./librocksdb_debug.a(write_thread.o): in function `rocksdb::WriteThread::SetState(rocksdb::WriteThread::Writer*, unsigned char)':
/home/adamretter/rocksdb/db/write_thread.cc:222: undefined reference to `__atomic_compare_exchange_1'
collect2: error: ld returned 1 exit status
make: *** [Makefile:1449: db_test] Error 1
```

Pull Request resolved: #9366

Reviewed By: jay-zhuang

Differential Revision: D34377664

Pulled By: mrambacher

fbshipit-source-id: c86f9d0cd1cb0c18de72b06f1bf5847f23f51118
dinoallo added a commit to dinoallo/gentoo that referenced this pull request Mar 15, 2022
dinoallo added a commit to dinoallo/gentoo that referenced this pull request Mar 15, 2022
dinoallo added a commit to dinoallo/gentoo that referenced this pull request Mar 15, 2022
dinoallo added a commit to dinoallo/gentoo that referenced this pull request Mar 15, 2022
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Mar 19, 2022
See: https://bugs.gentoo.org/834855#c3
Related upstream PR: facebook/rocksdb#9215

Bug: https://bugs.gentoo.org/834855
Signed-off-by: Yun Pan <dinoallo@netc.it>
Signed-off-by: Yixun Lan <dlan@gentoo.org>
@playground
Copy link

How can I install and test nodejs on risc-v Fedora?

curl -sL https://rpm.nodesource.com/setup_16.x | sudo -E bash - 

## Installing the NodeSource Node.js 16.x repo...


## Inspecting system...

+ rpm -q --whatprovides redhat-release || rpm -q --whatprovides centos-release || rpm -q --whatprovides cloudlinux-release || rpm -q --whatprovides sl-release || rpm -q --whatprovides fedora-release
+ uname -m

## You don't appear to be running a supported machine architecture: riscv64. Please contact NodeSource at https://github.com/nodesource/distributions/issues if you think this is incorrect or would like your architecture to be considered for support. ```

1 similar comment
@playground
Copy link

How can I install and test nodejs on risc-v Fedora?

curl -sL https://rpm.nodesource.com/setup_16.x | sudo -E bash - 

## Installing the NodeSource Node.js 16.x repo...


## Inspecting system...

+ rpm -q --whatprovides redhat-release || rpm -q --whatprovides centos-release || rpm -q --whatprovides cloudlinux-release || rpm -q --whatprovides sl-release || rpm -q --whatprovides fedora-release
+ uname -m

## You don't appear to be running a supported machine architecture: riscv64. Please contact NodeSource at https://github.com/nodesource/distributions/issues if you think this is incorrect or would like your architecture to be considered for support. ```

@XieJiSS
Copy link
Contributor Author

XieJiSS commented Mar 24, 2022

Hi @playground, this is a bit offtopic, but AFAIK you need to grab the nodejs 16.14.+ source code and compile it manually:

$ sudo dnf install python3 gcc-c++ make python3-pip
$ curl -O https://nodejs.org/dist/v16.14.2/node-v16.14.2.tar.xz
$ tar -xf node-v16.14.2.tar.xz
$ cd node-v16.14.2
$ ./configure
$ make CFLAGS="-fno-strict-aliasing $CFLAGS" CXXFLAGS="-fno-strict-aliasing $CXXFLAGS" -j4

Let's stop here and kindly keep the thread clean :-)

@siying
Copy link
Contributor

siying commented May 6, 2022

@hermanlee @lth any comment on this?

@facebook-github-bot
Copy link
Contributor

@XieJiSS has updated the pull request. You must reimport the pull request before landing.

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

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

Thanks @XieJiSS for the PR and @adamretter for verifying it.

@facebook-github-bot
Copy link
Contributor

@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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

Successfully merging this pull request may close these issues.

None yet

6 participants