Skip to content

Use reinterpret instead of c-style casting for GCC#22938

Merged
alalek merged 2 commits intoopencv:4.xfrom
Xxfore:4.x
Jan 11, 2023
Merged

Use reinterpret instead of c-style casting for GCC#22938
alalek merged 2 commits intoopencv:4.xfrom
Xxfore:4.x

Conversation

@Xxfore
Copy link
Copy Markdown
Contributor

@Xxfore Xxfore commented Dec 9, 2022 •
edited by alalek
Loading

Adjust for GCC about type cast.

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 another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the 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
force_builders=Custom
Xbuild_image:Custom=riscv-gcc
Xbuild_image:Custom=riscv-gcc-rvv
build_image:Custom=riscv-gcc-rvv-128
Xbuild_image:Custom=riscv-clang
Xbuild_image:Custom=riscv-clang-rvv
Xbuild_image:Custom=riscv-clang-rvv-128
test_modules:Custom=core,imgproc,dnn
buildworker:Custom=linux-1
test_timeout:Custom=1200
build_contrib:Custom=OFF

@asmorkalov asmorkalov requested a review from alalek December 12, 2022 07:10
alalek
alalek reviewed Dec 13, 2022
View reviewed changes
modules/core/include/opencv2/core/hal/intrin_rvv.hpp Outdated
{
uint64 ptr[2] = {0x0908060504020100, 0xFFFFFF0F0E0D0C0A};
return v_int8x16((vint8m1_t)vrgather_vv_u8m1((vuint8m1_t)vint8m1_t(vec), (vuint8m1_t)vle64_v_u64m1(ptr, 2), 16));
return v_int8x16(vreinterpret_v_u8m1_i8m1(vrgather_vv_u8m1(vreinterpret_v_i8m1_u8m1(vint8m1_t(vec)), vreinterpret_v_u64m1_u8m1(vle64_v_u64m1(ptr, 2)), 16)));
Copy link
Copy Markdown
Member

@alalek alalek Dec 13, 2022

Choose a reason for hiding this comment

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

Which compiler are you using?

/build/precommit_custom_linux/4.x/opencv/modules/core/include/opencv2/core/hal/intrin_rvv.hpp: In function 'cv::hal_baseline::v_int16x8 cv::hal_baseline::v_pack_triplets(const v_int16x8&)':
/build/precommit_custom_linux/4.x/opencv/modules/core/include/opencv2/core/hal/intrin_rvv.hpp:2833:65: error: 'vreinterpret_v_i16m1_u8m1' was not declared in this scope; did you mean 'vreinterpret_v_u16m1_u8m1'?
 2833 |     return v_int16x8(vreinterpret_v_u8m1_i16m1(vrgather_vv_u8m1(vreinterpret_v_i16m1_u8m1(vint16m1_t(vec)), vreinterpret_v_u64m1_u8m1(vle64_v_u64m1(ptr, 2)), 16)));
      |                                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                                 vreinterpret_v_u16m1_u8m1
/build/precommit_custom_linux/4.x/opencv/modules/core/include/opencv2/core/hal/intrin_rvv.hpp:2833:22: error: 'vreinterpret_v_u8m1_i16m1' was not declared in this scope; did you mean 'vreinterpret_v_u8m1_u16m1'?
 2833 |     return v_int16x8(vreinterpret_v_u8m1_i16m1(vrgather_vv_u8m1(vreinterpret_v_i16m1_u8m1(vint16m1_t(vec)), vreinterpret_v_u64m1_u8m1(vle64_v_u64m1(ptr, 2)), 16)));
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~
      |                      vreinterpret_v_u8m1_u16m1

Compiler: https://github.com/riscv-collab/riscv-gnu-toolchain/tree/rvv-next

Copy link
Copy Markdown
Contributor

@mshabunin mshabunin Dec 13, 2022

Choose a reason for hiding this comment

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

It seems that this patch fixes compilation with T-Head toolchain 2.6.1 (https://occ.t-head.cn/community/download?id=4090445921563774976). I've tested it (ported to 4.x) and it works now:

PATH=/opt/Xuantie-900-gcc-linux-5.10.4-glibc-x86_64-V2.6.1/bin:${PATH} \
cmake -GNinja \
        -DCMAKE_BUILD_TYPE=Release \
        -DCMAKE_TOOLCHAIN_FILE=/opencv/platforms/linux/riscv64-gcc.toolchain.cmake \
        -DRISCV_RVV_SCALABLE=OFF \
        -DCPU_BASELINE=RVV \
        -DCPU_RVV_FLAGS_ON=-march=rv64gcv1p0 \
        -DBUILD_SHARED_LIBS=OFF \
        -DWITH_OPENCL=OFF \
    /opencv

Tested with T-Head qemu (https://occ.t-head.cn/community/download?id=4108987827910414336), it seems that the c908v core supports RVV 1.0 unlike c906 and c910v:

OPENCV_TEST_DATA_PATH=/opencv_extra/testdata \
/opt/qemu/bin/qemu-riscv64 \
	-cpu c908v \
	-L /opt/Xuantie-900-gcc-linux-5.10.4-glibc-x86_64-V2.6.1/sysroot/ \
    ./bin/opencv_test_core --gtest_filter=*HAL*:*intrin*

Copy link
Copy Markdown
Contributor

@mshabunin mshabunin Dec 13, 2022

Choose a reason for hiding this comment

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

I can see that T-Head compiler defines #define __THEAD_VERSION__ "2.6.1" macro, perhaps we could modify the patch so that it would work with both mainline and T-Head toolchains.

Copy link
Copy Markdown
Member

@alalek alalek Dec 22, 2022

Choose a reason for hiding this comment

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

@Xxfore Ping

Please dump riscv64-unknown-linux-gnu-g++ -march=rv64gcv -dM -E - < /dev/null output from your compiler.

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Jan 10, 2023

I've updated the code to fix build with mainline gcc. Should work with all toolchains now: GCC from riscv-collab, GCC by T-Head v2.6.1, LLVM.

@Xxfore , @alalek please check your toolchains if possible.

alalek
alalek approved these changes Jan 11, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

  • Confirmed compilation with GCC from rvv-next and RISCV_RVV_SCALABLE=OFF

@alalek alalek merged commit ef0fcb9 into opencv:4.x Jan 11, 2023
@alalek alalek mentioned this pull request Jan 18, 2023
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
@Xxfore @mshabunin
Merge pull request opencv#22938 from Xxfore:4.x
2902127 
Use reinterpret instead of c-style casting for GCC

Co-authored-by: Xu Zhang <xu.zhang@hexintek.com>
Co-authored-by: Maksim Shabunin <maksim.shabunin@gmail.com>
geversonsto pushed a commit to stodev-com-br/opencv that referenced this pull request Jun 3, 2023
@Xxfore @mshabunin @geversonsto
Merge pull request opencv#22938 from Xxfore:4.x
9e46d45 
Use reinterpret instead of c-style casting for GCC

Co-authored-by: Xu Zhang <xu.zhang@hexintek.com>
Co-authored-by: Maksim Shabunin <maksim.shabunin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshabunin mshabunin mshabunin left review comments

@alalek alalek alalek approved these changes

Assignees

@alalek alalek

Labels

category: build/install platform: riscv

Projects

None yet

Milestone

4.8.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Xxfore @mshabunin @alalek @asmorkalov