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

Remove cuda cub #110

Merged
merged 6 commits into from
Aug 12, 2019
Merged

Remove cuda cub #110

merged 6 commits into from
Aug 12, 2019

Conversation

griwodz
Copy link
Member

@griwodz griwodz commented Aug 2, 2019

This PR is meant to replace #103 . That PR attempts to fix a thrust::sort linker problem for sm5X that is due to an old bug in Thrust.

This is a simpler solution: CUB is removed entirely, the Thrust linker bug is fixed in an unexpected way: instead of fixing thrust::sort, just provide a StrictWeakOrdering operator implemented in a way that guarantees that it doesn't use any memory.

@griwodz griwodz mentioned this pull request Aug 2, 2019
@griwodz
Copy link
Member Author

griwodz commented Aug 5, 2019

AppVeyor could no longer find packages. I followed recommendations for upgrading AppVeyor and hope that I didn't break anything badly.

Copy link
Member

@simogasp simogasp left a comment

Choose a reason for hiding this comment

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

I don't know how serious is the problem with cuda 7 and some GPUs, we can make it more explicit maybe as i suggested

src/cctag/cuda/frame_07d_vote_if.cu Outdated Show resolved Hide resolved
@simogasp simogasp added the cuda label Aug 8, 2019
@simogasp
Copy link
Member

simogasp commented Aug 8, 2019

LGTM, if someone manages to build it on a windoze system just to be sure we can merge.

@simogasp simogasp self-requested a review August 8, 2019 20:22
@griwodz
Copy link
Member Author

griwodz commented Aug 9, 2019

LGTM, if someone manages to build it on a windoze system just to be sure we can merge.

Well, it was "fixed" by asking AppVeyor for the "Visual Studio 2017" image instead of the "Visual Studio 2015" image. Other people have used "Previous Visual Studio 2015" as image. I hope that's good enough for now?

# Conflicts:
#	CMakeLists.txt
@griwodz griwodz merged commit 914c6f3 into develop Aug 12, 2019
@griwodz griwodz deleted the remove_cudaCub branch August 12, 2019 08:10
julianrendell added a commit to julianrendell/CCTag that referenced this pull request May 21, 2020
Came across this- PR alicevision#110 .

So I think this section is no longer correct. But searching the repo shows a couple of places where the flag is used in the cmake files- not sure if that needs cleanup, or I'm mistaken by the title of PR alicevision#110 .
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.

2 participants