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

AVX2 makes PCL crash #5806

Open
Vakarian15 opened this issue Sep 11, 2023 · 14 comments
Open

AVX2 makes PCL crash #5806

Vakarian15 opened this issue Sep 11, 2023 · 14 comments
Labels
kind: bug Type of issue status: triage Labels incomplete

Comments

@Vakarian15
Copy link

Vakarian15 commented Sep 11, 2023

Describe the bug

I am trying to downsample a pcl::PointXYZRGBNormal PointCloud using a VoxelGrid filter.
AVX2 makes my program crash, I tried it with 1.12.0 and 1.13.1, both of them have the same problem.

To Reproduce

int main()
{
	pcl::PointCloud<pcl::PointXYZRGBNormal>::Ptr cloud(new pcl::PointCloud<pcl::PointXYZRGBNormal>());
	pcl::PointCloud<pcl::PointXYZRGBNormal>::Ptr cloud_filtered(new pcl::PointCloud<pcl::PointXYZRGBNormal>());

	// Fill in the cloud data
	pcl::PCDReader reader;
	reader.read("test.pcd", *cloud); 

	// Create the filtering object
	pcl::VoxelGrid<pcl::PointXYZRGBNormal> sor;
	sor.setInputCloud(cloud);
	sor.setLeafSize(0.01f, 0.01f, 0.01f);
	sor.filter(*cloud_filtered);
	return (0);
}

If I disable AVX2, it works. If I replace pcl::PointCloud<pcl::PointXYZRGBNormal> with pcl::PCLPointCloud2, it works. So I'm not sure it is a problem of pcl::PointXYZRGBNormal or pcl::VoxelGrid.
Here is the sample project including the dataset: PCLTest.zip

Screenshots/Code snippets
Stack track
image

image

Your Environment (please complete the following information):

  • OS: Win11
  • Compiler: CMake, MSVC 14.27
  • PCL Version: 1.12.0 and 1.13.1 download from vcpkg

Additional context

In 1.13.1, both pcl::PointXYZRGBNormal and pcl::VoxelGrid have PCL_MAKE_ALIGNED_OPERATOR_NEW macro. So I have no idea what the problem is. Thanks in advance.

@Vakarian15 Vakarian15 added kind: bug Type of issue status: triage Labels incomplete labels Sep 11, 2023
@larshg
Copy link
Contributor

larshg commented Sep 12, 2023

@Vakarian15 can you try find the configure log file from VCPKG build of PCL - maybe we can see something in the log, whether PCL was build with AVX support or not.
If its not and you enable it, in your project, it can fail with these errors.

@Vakarian15
Copy link
Author

Vakarian15 commented Sep 12, 2023

@Vakarian15 can you try find the configure log file from VCPKG build of PCL - maybe we can see something in the log, whether PCL was build with AVX support or not. If its not and you enable it, in your project, it can fail with these errors.

@larshg Thank you for your swift response. Here is the configure log
config-x64-windows-CMakeCache.txt.log
It has PCL_ENABLE_AVX:BOOL=ON. Not sure whether this means it has AVX series support or only support AVX. But I still get the same error when I enable AVX instead of AVX2 in my project

@larshg
Copy link
Contributor

larshg commented Sep 12, 2023

That was the cmake cache and yes, the variable you refer to, is that it should test for AVX(2), but I can't find any "HAVE_AVX" or the like.

I was requesting the configuration output when running cmake.
It should be like:
C:\vcpkg\buildtrees\pcl\install-x64-windows-rel-out.log
Or
C:\vcpkg\buildtrees\pcl\configure-x64-windows-rel-out.log

Its example from the azure pipline, but hopefully you can find similiar :-)

@larshg
Copy link
Contributor

larshg commented Sep 12, 2023

I can find
HAVE_SSE4_2_EXTENSIONS:INTERNAL=1
Which indicates you have at least sse 4.2.

@Vakarian15
Copy link
Author

Vakarian15 commented Sep 12, 2023

Here are the files
config-x64-windows-out.log
install-x64-windows-rel-out.log
And you are right, I cannot find AVX flags.
I tired to create a overlay port, by adding -DPCL_ENABLE_AVX=ON to profile.cmake. Delete buildtrees/pcl folder and install pcl with the overlay port
image
And here is the new config log
config-x64-windows-out.log
-DPCL_ENABLE_AVX=ON was passed to cmake successfully.
But the problem is not fixed. Could you please have a look? @larshg Is there something I did wrong?

@larshg
Copy link
Contributor

larshg commented Sep 13, 2023

Ahh, vcpkg have a patch for SSE https://github.com/microsoft/vcpkg/blob/master/ports/pcl/fix-check-sse.patch

But not for AVX and they don't parse default CXX settings, so it will never check for AVX instructions.
You can try make a patch similiar to the SSE one and add this to the vcpkg portfile.

@Vakarian15
Copy link
Author

Ahh, vcpkg have a patch for SSE https://github.com/microsoft/vcpkg/blob/master/ports/pcl/fix-check-sse.patch

But not for AVX and they don't parse default CXX settings, so it will never check for AVX instructions. You can try make a patch similiar to the SSE one and add this to the vcpkg portfile.

I see, thanks for your time!

@Vakarian15
Copy link
Author

Vakarian15 commented Sep 13, 2023

I made a patch for AVX
Screenshot 2023-09-13 111549
Then add the patch to protfile.cmake
Screenshot 2023-09-13 111637
Screenshot 2023-09-13 111659
Then remove pcl from vcpkg and reinstall with overlay ports
And here is the new log file
config-x64-windows-out.log
It says "Performing Test HAVE_AVX2 - Success" in the log.
But the problem is not fixed.

@larshg
Copy link
Contributor

larshg commented Sep 13, 2023

And you have enabled AVX in your project as well as copied new dlls etc?

@Vakarian15
Copy link
Author

Yes, I even copied the whole vcpkg\installed\x64-windows\bin folder.
I tried allinone installer, it doesn't work.
I also tired to build from source with -DPCL_ENABLE_AVX=ON, it doesn't work either.

@Aposhian
Copy link

Aposhian commented Sep 28, 2023

I was able to get AVX2 to work by using only custom point types with EIGEN_ALIGN32 (and then making sure to define PCL_NO_PRECOMPILE). It would be nice to have preprocessor directives for PCL that made it easier to change max alignment, similar to how Eigen does it upstream.

@Neumann-A
Copy link

I find it funny that there are two connected issues so close together. I think #5805 is the problem. PCL is using EIGEN_ALIGN16 which probably does not work with /arch:AVX2. I would say: Let the compiler decide the alignment or make it dependent on the target architecture.

@mvieth
Copy link
Member

mvieth commented Feb 9, 2024

Hi, thank you for your suggestions, but I tested and I see no reason to assume that EIGEN_ALIGN16 causes the problem:

  • first of all, I could reproduce the issue with the original code (pcl::PointXYZRGBNormal, no PCL_NO_PRECOMPILE, PCL installed via vcpkg)
  • if I simply add #define PCL_NO_PRECOMPILE at the top, it works
  • if I add a custom point type that is basically a clone of pcl::PointXYZRGBNormal (including EIGEN_ALIGN16), and add define PCL_NO_PRECOMPILE (necessary for custom point type), it works

Same as Lars, I think the problem is likely that vcpkg built PCL without AVX/AVX2, so the precompiled classes use a "vanilla" malloc instead of Eigen's custom aligned malloc. When the user code is compiled with AVX/AVX2, Eigen's aligned_free is used (not a "vanilla" free). The mismatch between the "vanilla" malloc and Eigen's aligned_free causes the problem. When using PCL_NO_PRECOMPILE, the PCL code is compiled together with the user code, and all flags and architecture options are consistent.
Adding a patch for AVX (like the patch that vcpkg has for SSE) is a good first step, but I believe it did not take effect yet because there is a second check for "${CMAKE_CXX_FLAGS}" STREQUAL "${CMAKE_CXX_FLAGS_DEFAULT}" : https://github.com/PointCloudLibrary/pcl/blob/master/CMakeLists.txt#L150 So AVX and AVX2 are found, but the flags are not passed to CMAKE_CXX_FLAGS.
I will make some more tests, and then create a PR for PCL, and perhaps also submit a patch to vcpkg.

@Aposhian
Copy link

As another data point, I encountered this bug on Linux with installing PCL binaries from apt, so it isn't just vcpkg. I haven't tried using the built-in EIGEN_ALIGN16 while simply specifying PCL_NO_PRECOMPILE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Type of issue status: triage Labels incomplete
Projects
None yet
Development

No branches or pull requests

5 participants