Page MenuHomePhabricator

[libcxx] <experimental/simd> implementation
Needs ReviewPublic

Authored by Joy12138 on Dec 6 2022, 5:48 AM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

Details of the patch: https://github.com/plctlab/llvm-project/blob/simd_for_upstream/README.md
Co-authored-by:
Yiliang He - <QuarticCat@protonmail.com>
Yi Zhang - <zhangyi216@mails.ucas.ac.cn>
Haolin Pan - <panhaolin21@mails.ucas.ac.cn>
Jiatai He - <jiatai2021@iscas.ac.cn>
Heda Chen - <marcythm@gmail.com>
Haichuan Hu - <huhaichuan0704@126.com>
Haohang Shi - <shyhot@outlook.com>

Diff Detail

Event Timeline

Joy12138 created this revision.Dec 6 2022, 5:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2022, 5:48 AM
Herald added a subscriber: miyuki. · View Herald Transcript
Joy12138 requested review of this revision.Dec 6 2022, 5:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2022, 5:48 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Thanks a lot for this patch! I had a short look at it. I will try to do a review next weekend. I first want to review the code itself and later on the "nitty" details.

philnik added a subscriber: philnik.Dec 6 2022, 8:28 AM

Thanks for working on this! It would be great to have a proper implementation of the parallelism TS. Unfortunately, the patch as-is is pretty much impossible to review because of the size. I've got a few high-level comments, but please split this up into smaller patches.

libcxx/include/CMakeLists.txt
514

These should be in experimental/__simd.

libcxx/test/std/experimental/simd/simd.casts/concat.pass.cpp
147–152

All the test main()s have to return 0; to support (semi-)freestanding environments.

153

Please make sure you have a newline at the end of the files.

Joy12138 added a comment.EditedDec 6 2022, 11:41 PM

@philnik @Mordante Hi reviewers! Thank you very much for your attention to this patch! There are exactly still many issues with this patch. In addition to the problems you mentioned, there are also problems with the logic of some tests. And there are still 4 tests that failed on arm64. And We have not run the test in Aarch64 and PPC environments. I believe that fixing these issues is necessary before the patch is merged.

I will split the patch into multiple smaller patches and resubmit them later. And also check and fix all these issues in the process. This may take several weeks. I suggest you review the new patch set later. Of course, we are also glad if you have time to review this patch and give us more suggestions.

@philnik @Mordante Hi reviewers! Thank you very much for your attention to this patch! There are exactly still many issues with this patch. In addition to the problems you mentioned, there are also problems with the logic of some tests. And there are still 4 tests that failed on arm64. And We have not run the test in Aarch64 and PPC environments. I believe that fixing these issues is necessary before the patch is merged.

I will split the patch into multiple smaller patches and resubmit them later. And also check and fix all these issues in the process. This may take several weeks. I suggest you review the new patch set later. Of course, we are also glad if you have time to review this patch and give us more suggestions.

Thanks for the update. When possible, it would be great if you can upload one smaller patch when it's ready. Then we can review it and look whether the style matches libc++'s style.