Replace allocator and vector classes with alias templates #1221
Conversation
There was a problem hiding this comment.
Nice, this was on my todo list now that C++03 is gone 👍 The patch is very appreciated!
This still needs a bit of work to compile, see the inline comments. #1163 (which I will finish and get merged this week) describes how to build and run the Thrust tests.
Also -- since you're tackling C++11 modernization stuff, just be aware of #1152 so we aren't duplicating work.
|
@allisonvacanti I'll tackle those requests. My concern is that this conflicts with #1152 because @brycelelbach modified the same files I modified but simply removed the CPP11 macros instead of replacing the classes with alias templates altogether. Additionally, I'm curious as to what you do to build it and see the errors that you see because from my end things were compiling cleanly. Would like to align my build process with what you have so we don't get these issues in the future. |
That's ok, 1152 is a rough pass that just removes the now-obsolete preprocessor checks -- your patch is a better solution for these files. We'll rebase 1152 once we get back to working on it.
I have a dedicated workstation set up to build all 36 combinations of HOST={cpp,omp,tbb} DEVICE={cpp,omp,tbb,cuda} DIALECT={11,14,17} on GCC 7.5.0 using the multiconfig cmake options. I use this as a temporary measure until we have better CI in place. I need to update the docs to describe multiconfig, but the linked file lays out the options and underlying logic. My CMake options: It can take up to 12 hours to fully build this on good hardware, so you'll probably want to scale things back or just stick with the single configuration build ( Alternatively, configure multiconfig and just compile/test a single configuration with (for example) The failing configurations (thrust.host.device.dialect) are the non-zero exit status ones below. The hit/miss stats can be ignored, they're for ccache. |
mfrancis95
force-pushed
the
cpp11
branch
2 times, most recently
from
1dec8ba
to
6132212
Compare
|
@allisonvacanti I force-pushed the corrections. All vector.inl files have been deleted. |
|
Looks good to me, just need to fix up the missing newlines at the ends of the Rerunning local tests now. |
|
Rebased on current main and addressed the newlines while launching CI. Perforce CL 28805798 |
alliepiper
added
testing: gpuCI passed
Delete /detail/vector.inl files since they are no longer needed. Files affected: - thrust/system/cpp/memory.h - thrust/system/cpp/vector.h - thrust/system/cuda/memory.h - thrust/system/cuda/vector.h - thrust/system/omp/memory.h - thrust/system/omp/vector.h - thrust/system/tbb/memory.h - thrust/system/tbb/vector.h
|
new CL after rebase: 28854727 |
Files affected: