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.

Sign up for GitHub

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

Jump to bottom

Intel fixes #2432

Merged
merged 2 commits into from
Sep 12, 2018
Merged

Intel fixes #2432

merged 2 commits into from
Sep 12, 2018

Conversation

svenevs
Copy link
Contributor

@svenevs svenevs commented Sep 11, 2018

Fixes DEPRECATED macros and _mm_malloc trauma with GCC. See this SO post for the _mm_malloc thing. Only tested on Linux, but this should be OK for macOS and Windows, but I can't test those with the intel compilers.

This was referenced Sep 11, 2018
Merged
Closed
SergioRAgostinho
SergioRAgostinho reviewed Sep 11, 2018
View reviewed changes
Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I would revise your preprocessor macros to use

#if
#elif
#else
#endif

appropriately. You can still enforce the order you need.

@SergioRAgostinho SergioRAgostinho added this to the pcl-1.9.0 milestone Sep 11, 2018
@SergioRAgostinho SergioRAgostinho added the needs: author reply Specify why not closed/merged yet label Sep 11, 2018
@svenevs
Copy link
Contributor Author

svenevs commented Sep 11, 2018

This should be much cleaner: 12efd4f

@svenevs
Copy link
Contributor Author

svenevs commented Sep 11, 2018

P.S. in C++14 we get [[deprecated]] 🙃

@taketwo taketwo removed the needs: author reply Specify why not closed/merged yet label Sep 11, 2018
SergioRAgostinho
SergioRAgostinho approved these changes Sep 11, 2018
View reviewed changes
Copy link
Member

@SergioRAgostinho SergioRAgostinho left a comment

Choose a reason for hiding this comment

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

To merge once the Windows CI gets through.

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Sep 12, 2018 •
edited
Loading

The Windows CI is failing with a legit test although it shouldn't be related whatsoever with this PR.

@svenevs at which commit did you branch out from master?

Edit: Either box search has a bug or a zero probability event just happened.

@taketwo
Copy link
Member

taketwo commented Sep 12, 2018

This test failure is reproducible. I pumped up test_runs in that test to 1 million and hit the same assertion. But it has nothing to do with this PR anyway.

@taketwo taketwo merged commit 62809fb into PointCloudLibrary:master Sep 12, 2018
@svenevs
Copy link
Contributor Author

svenevs commented Sep 12, 2018

Sorry for the lag. This seems resolved now though :)

@svenevs svenevs deleted the intel-fixes branch September 12, 2018 14:23
@SergioRAgostinho SergioRAgostinho mentioned this pull request Sep 25, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SergioRAgostinho SergioRAgostinho SergioRAgostinho approved these changes

@taketwo taketwo taketwo approved these changes

Assignees
No one assigned
Labels
module: cmake module: common
Projects
None yet
Milestone
pcl-1.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@svenevs @SergioRAgostinho @taketwo