Skip to content

Master riscv 0.7.1#19778

Merged
alalek merged 10 commits intoopencv:masterfrom
damonyu1989:master-riscv-0.7.1
May 25, 2021
Merged

Master riscv 0.7.1#19778
alalek merged 10 commits intoopencv:masterfrom
damonyu1989:master-riscv-0.7.1

Conversation

@damonyu1989
Copy link
Copy Markdown
Contributor

@damonyu1989 damonyu1989 commented Mar 25, 2021 •
edited
Loading

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@damonyu1989
Copy link
Copy Markdown
Contributor Author

damonyu1989 commented Mar 25, 2021

mkdir build
cd build
cmake .. -DCMAKE_TOOLCHAIN_FILE="../ platforms/linux/riscv64-0.7.1-gcc.toolchain.cmake" -DWITH_OPENCL=OFF -DBUILD_opencv_calib3d=ON -DBUILD_ZLIB=ON -DBUILD_PNG=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_WITH_INSTALL_RPATH=1 -DCMAKE_INSTALL_PREFIX=../install
make

@damonyu1989
Copy link
Copy Markdown
Contributor Author

damonyu1989 commented Mar 25, 2021

The toolchian can be found in https://github.com/damonyu1989/Tools.

@alalek
Copy link
Copy Markdown
Member

alalek commented Mar 25, 2021

Do NOT re-open PRs. Update your source branch instead.

refs: #18394 #19768

@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented Mar 26, 2021

@damonyu1989, thank you!
as @alalek said, since the changes are rather local, can you please pull the latest master from opencv repository, make a fresh branch and put your code there?

also, since in OpenCV there is already some RISC-V acceleration, there should be some way to select your version over the other one? Eventually the two implementations should be merged together, but at least they should not conflict with each other.

@vpisarev vpisarev self-assigned this Mar 26, 2021
@vpisarev vpisarev self-requested a review March 26, 2021 06:46
@damonyu1989
Copy link
Copy Markdown
Contributor Author

damonyu1989 commented Mar 26, 2021

Dear vadim, My code are merged to the latest master commit. In my opinions, the PR we pushed, is the version of 0.7.1. So we add a new file for this version .

@damonyu1989
Copy link
Copy Markdown
Contributor Author

damonyu1989 commented Mar 27, 2021

@vpisarev

@asenyaev
Copy link
Copy Markdown
Contributor

asenyaev commented Apr 8, 2021

jenkins cn please retry a build

@damonyu1989 damonyu1989 closed this Apr 9, 2021
@damonyu1989 damonyu1989 reopened this May 14, 2021
@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented May 19, 2021

@damonyu1989, I've sent you the version of intrin_rvv071.hpp with some GCC warnings fixed. Please, update it. Also, please, remove trailing whitespaces, which our doc builder complains about, and squash commits into one.

@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented May 25, 2021

@alalek, the patch is ready to be merged, I think 👍 I tested it my machine under QEMU, works well, all opencv_test_core tests pass. Could you please squash the commits and merge it?

alalek
alalek reviewed May 25, 2021
View reviewed changes
modules/core/include/opencv2/core/hal/intrin_rvv071.hpp Outdated
// This software is provided by the copyright holders and contributors "as is" and
// any express or implied warranties, including, but not limited to, the implied
// warranties of merchantability and fitness for a particular purpose are disclaimed.
// In no event shall the Intel Corporation or contributors be liable for any direct,
Copy link
Copy Markdown
Member

@alalek alalek May 25, 2021

Choose a reason for hiding this comment

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

Please use short OpenCV license header with "authors" extra comments.

Copy link
Copy Markdown
Contributor Author

@damonyu1989 damonyu1989 May 25, 2021

Choose a reason for hiding this comment

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

@damonyu1989, is it fine with you if the license header in intrin_rvv071.hpp is replaced with

// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html

// Copyright (C) 2015, PingTouGe Semiconductor Co., Ltd., all rights reserved.

??
I think it is ok for us.

@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented May 25, 2021

@damonyu1989, is it fine with you if the license header in intrin_rvv071.hpp is replaced with

// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html

// Copyright (C) 2015, PingTouGe Semiconductor Co., Ltd., all rights reserved.

??

@vpisarev vpisarev requested a review from alalek May 25, 2021 13:59
@alalek alalek merged commit 5f637e5 into opencv:master May 25, 2021
@hanliutong hanliutong mentioned this pull request May 30, 2021
Merged
6 tasks
@alalek alalek mentioned this pull request Jun 4, 2021
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@damonyu1989 @vpisarev
Merge pull request opencv#19778 from damonyu1989:master-riscv-0.7.1
915dc98 
* Add the support for riscv64 vector 0.7.1.

* fixed GCC warnings

* cleaned whitespaces

* Remove the worning by the use of internal API of compiler.

* Update the license header.

* removed trailing whitespaces

Co-authored-by: Vadim Pisarevsky <vadim.pisarevsky@me.com>
Co-authored-by: yulj <linjie.ylj@alibaba-inc.com>
Co-authored-by: Vadim Pisarevsky <vadim.pisarevsky@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek left review comments

@vpisarev vpisarev vpisarev approved these changes

Assignees

@vpisarev vpisarev

Labels

platform: riscv

Projects

None yet

Milestone

4.5.3

Development

Successfully merging this pull request may close these issues.

4 participants

@damonyu1989 @alalek @vpisarev @asenyaev