Move paddle/majel/* to paddle/{platform,framework/*

[wangkuiyi]

Fixes #2507
Fixes #2508

[JiayiFeng]

LGTM

[gangliao]

Just wondering, how it find head file boost/variant.h if you comment include_directories(${Boost_INCLUDE_DIRS})

[wangkuiyi]

I think that's because we install libboost-dev within Docker image, and we got boost header files in standard place -- /usr/share/boost/include or somewhere.

[wangkuiyi]

I am going to file a subsequent PR to remove our dependency to boost -- substitute boost::variant by std::variant.

[gangliao]

But we should consider this seriously. std::variant only supported by c++17.

[gangliao]

@wangkuiyi GCC 7 has std::variant, I don't think CUDA NVCC supports GCC 7 very well.

[wangkuiyi]

I got it. Let me add the check of the existence of Boost. Thanks for the remind!

[gangliao]

why we need this new clang-format file? We only have one in Paddle/.clang-format.

[gangliao]

Shall we also use cpplint as a complementary ? Because it already in cmake/clint.cmake, maybe we can invokes the wrapper function add_style_check_target in generic functions.

[wangkuiyi]

/.clang-format was created by @reyoung to fit existing code style. However, we'd like to follow strict Google C++ style, so that during code review we could paste links into Google style's online doucment to help use explain pros and cons of programming choices.

clang-format works by looking for .clang-format in the nearest parent directory of the source file. So, as we put the new .clang-format in paddle/framework, clang-format would check paddle/framework/ddim.h and other source files following Google C++ style.

[wangkuiyi]

I am only enforcing Google C++ style for new directories. Do you mean we call cpplint to check new code? I'd love to have it, but it seems cannot reformat the code like clang-format. Do we really need it?

[gangliao]

Not sure, just remind here.

It will scan [static analysis] and trigger the error if your code is "out of the box", for instance, if you did not add // namespace platform comment at the end, aha, here is a buggy.

[gangliao]

asm("trap") is inconsistent in Majel. Because I found in abort.h, gdiamos implemented

__device__ inline void abort_gpu_kernel(const char* message) { printf("Aborted GPU Kernel: `%s` .\n", message); #if !defined(__APPLE__) asm("trap;"); #endif }

However, this PADDLE_ASSERT macro (implemented by others) only works when it runs on APPLE.

I guess maybe it's

!defined(__APPLE__) && defined(__CUDA_ARCH__) && !defined(NDEBUG) 

Not be Verified...

[wangkuiyi]

This PR's unit tests passed TeamCity CI. Does this mean that this macro also works with Linux+CUDA?

[wangkuiyi]

I created #2524 and will consult Greg tomorrow morning.

[gangliao]

Sounds cool!

Also, after reading this post, I guess the current code is correct and no need to modify.

CUDA now has a native assert function. Use assert(...). If its argument is zero, it will stop kernel execution and return an error. (or trigger a breakpoint if in CUDA debugging.)

Make sure to include "assert.h". Also, this requires compute capability 2.x or higher, and is
not supported on MacOS. For more details see CUDA C Programming Guide, Section B.16.

The programming guide also includes this example:

#include <assert.h> __global__ void testAssert(void) { int is_one = 1; int should_be_one = 0; // This will have no effect assert(is_one); // This will halt kernel execution assert(should_be_one); } int main(int argc, char* argv[]) { testAssert<<<1,1>>>(); cudaDeviceSynchronize(); return 0; }

[gangliao]

if(Boost_FOUND)
include_directories(${Boost_INCLUDE_DIRS})
add_subdirectory(platform)
add_subdirectory(framework)
endif()

We need this line to guarantee the compiler can find the header file.

[gangliao]

LGTM. We can merge this PR first, then adjust it.

For example, we need to consider the pure cpu version @QiJune .

#if !defined(PADDLE_ONLY_CPU) typedef boost::variant<GpuPlace, CpuPlace> Place; #else typedef boost::variant<CpuPlace> Place; #endif ... // may need more modification. 

That can help us to trigger the error during the period of compilation.