| Message ID | 1475088032-30321-1-git-send-email-jcmvbkbc@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello, Adding Samuel in Cc: there is a CMake issue below. On Wed, 28 Sep 2016 11:40:32 -0700, Max Filippov wrote: > The commit 4904c4c "package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC" > overrides CMAKE_CXX_FLAGS with the single -latomic, losing all ABI > CFLAGS that are passed there by default. This breaks build on xtensa > where ABI CFLAGS contain important code generation options. > > Append $(TARGET_CXXFLAGS) to CMAKE_CXX_FLAGS along with -latomic. > > Fixes: > http://autobuild.buildroot.net/results/7f1c96abd8fbb5b358a07100ab623316e9bb9dcd > http://autobuild.buildroot.net/results/e0c93d0f6d1da0d62d4dbba211a275bfe75e9645 > http://autobuild.buildroot.net/results/53e7e4b4b6a7b48b8012799d7507f7594dbf01b2 > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > package/opencv3/opencv3.mk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/package/opencv3/opencv3.mk b/package/opencv3/opencv3.mk > index 2529de9..10660a9 100644 > --- a/package/opencv3/opencv3.mk > +++ b/package/opencv3/opencv3.mk > @@ -12,7 +12,7 @@ OPENCV3_LICENSE_FILES = LICENSE > > # Uses __atomic_fetch_add_4 > ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y) > -OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic" > +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic" There is something fishy going on here. Indeed, support/misc/toolchainfile.cmake.in contains: set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") So it should normally always prefix CMAKE_CXX_FLAGS with TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets generated during your build? Does it contains the Xtensa specific CXXFLAGS ? Best regards, Thomas
Hi Thomas, On Thu, Sep 29, 2016 at 2:27 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Adding Samuel in Cc: there is a CMake issue below. > > On Wed, 28 Sep 2016 11:40:32 -0700, Max Filippov wrote: >> The commit 4904c4c "package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC" >> overrides CMAKE_CXX_FLAGS with the single -latomic, losing all ABI >> CFLAGS that are passed there by default. This breaks build on xtensa >> where ABI CFLAGS contain important code generation options. >> >> Append $(TARGET_CXXFLAGS) to CMAKE_CXX_FLAGS along with -latomic. >> >> Fixes: >> http://autobuild.buildroot.net/results/7f1c96abd8fbb5b358a07100ab623316e9bb9dcd >> http://autobuild.buildroot.net/results/e0c93d0f6d1da0d62d4dbba211a275bfe75e9645 >> http://autobuild.buildroot.net/results/53e7e4b4b6a7b48b8012799d7507f7594dbf01b2 >> >> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> >> --- >> package/opencv3/opencv3.mk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/package/opencv3/opencv3.mk b/package/opencv3/opencv3.mk >> index 2529de9..10660a9 100644 >> --- a/package/opencv3/opencv3.mk >> +++ b/package/opencv3/opencv3.mk >> @@ -12,7 +12,7 @@ OPENCV3_LICENSE_FILES = LICENSE >> >> # Uses __atomic_fetch_add_4 >> ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y) >> -OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic" >> +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic" > > There is something fishy going on here. Indeed, > support/misc/toolchainfile.cmake.in contains: > > set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") > > So it should normally always prefix CMAKE_CXX_FLAGS with > TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets > generated during your build? Does it contains the Xtensa specific > CXXFLAGS ? Yes, it does. opencv2 which has similar cmake build system but doesn't pass -DCMAKE_CXX_FLAGS builds correctly.
Hello, On Thu, 29 Sep 2016 09:14:41 -0700, Max Filippov wrote: > > There is something fishy going on here. Indeed, > > support/misc/toolchainfile.cmake.in contains: > > > > set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") > > > > So it should normally always prefix CMAKE_CXX_FLAGS with > > TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets > > generated during your build? Does it contains the Xtensa specific > > CXXFLAGS ? > > Yes, it does. So if it does, why is your patch necessary? I'm not following here. Do you mean that the Xtensa-specific flags are present in the toolchain.cmake file, but are in the end not used during the build ? > opencv2 which has similar cmake build system but doesn't pass > -DCMAKE_CXX_FLAGS builds correctly. OK. Samuel, can you advise here? Thanks! Thomas
On Fri, Sep 30, 2016 at 1:10 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > On Thu, 29 Sep 2016 09:14:41 -0700, Max Filippov wrote: >> > There is something fishy going on here. Indeed, >> > support/misc/toolchainfile.cmake.in contains: >> > >> > set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") >> > >> > So it should normally always prefix CMAKE_CXX_FLAGS with >> > TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets >> > generated during your build? Does it contains the Xtensa specific >> > CXXFLAGS ? >> >> Yes, it does. > > So if it does, why is your patch necessary? I'm not following here. > Do you mean that the Xtensa-specific flags are present in the > toolchain.cmake file, but are in the end not used during the build ? Right. The file host/usr/share/buildroot/toolchainfile.cmake exists and contains the following (correct) lines: set(CMAKE_C_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS") set(CMAKE_CXX_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") But the file build/opencv3-3.1.0/CMakeCache.txt that governs opencv3 build contains the following lines: //Buildroot CXXFLAGS CMAKE_CXX_FLAGS:STRING=-latomic //Buildroot CFLAGS CMAKE_C_FLAGS:STRING='-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ' Below is the output that I see during the failing opencv3 build: -- C++ flags (Release): -latomic -fsigned-char -W -Wall -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -Wformat -Werror=format-security -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow -Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor -fdiagnostics-show-option -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden -O3 -DNDEBUG -DNDEBUG -- C++ flags (Debug): -latomic -fsigned-char -W -Wall -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -Wformat -Werror=format-security -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow -Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor -fdiagnostics-show-option -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden -g -O0 -DDEBUG -D_DEBUG -- C Compiler: /home/jcmvbkbc/tmp/br/build-20160928-reproduce-opencv/host/usr/bin/xtensa-linux-gcc -- C flags (Release): -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os -fsigned-char -W -Wall -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -Wformat -Werror=format-security -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self -Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden -O3 -DNDEBUG -DNDEBUG -- C flags (Debug): -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os -fsigned-char -W -Wall -Werror=return-type -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point -Wformat -Werror=format-security -Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self -Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden -g -O0 -DDEBUG -D_DEBUG Notice the difference between the C and the C++ flags.
Hi all, On Fri, Sep 30, 2016 at 6:32 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: > On Fri, Sep 30, 2016 at 1:10 AM, Thomas Petazzoni > <thomas.petazzoni@free-electrons.com> wrote: >> On Thu, 29 Sep 2016 09:14:41 -0700, Max Filippov wrote: >>> > There is something fishy going on here. Indeed, >>> > support/misc/toolchainfile.cmake.in contains: >>> > >>> > set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") >>> > >>> > So it should normally always prefix CMAKE_CXX_FLAGS with >>> > TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets >>> > generated during your build? Does it contains the Xtensa specific >>> > CXXFLAGS ? >>> >>> Yes, it does. >> >> So if it does, why is your patch necessary? I'm not following here. >> Do you mean that the Xtensa-specific flags are present in the >> toolchain.cmake file, but are in the end not used during the build ? > > Right. The file > host/usr/share/buildroot/toolchainfile.cmake > exists and contains the following (correct) lines: > > set(CMAKE_C_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os > ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS") > set(CMAKE_CXX_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os > ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") > > But the file > build/opencv3-3.1.0/CMakeCache.txt > that governs opencv3 build contains the following lines: > > //Buildroot CXXFLAGS > CMAKE_CXX_FLAGS:STRING=-latomic > //Buildroot CFLAGS > CMAKE_C_FLAGS:STRING='-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE > -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ' > > Below is the output that I see during the failing opencv3 build: > > -- C++ flags (Release): -latomic -fsigned-char -W -Wall > -Werror=return-type -Werror=non-virtual-dtor -Werror=address > -Werror=sequence-point -Wformat -Werror=format-security > -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow > -Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor > -fdiagnostics-show-option -pthread -fomit-frame-pointer > -ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden > -O3 -DNDEBUG -DNDEBUG > -- C++ flags (Debug): -latomic -fsigned-char -W -Wall > -Werror=return-type -Werror=non-virtual-dtor -Werror=address > -Werror=sequence-point -Wformat -Werror=format-security > -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow > -Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor > -fdiagnostics-show-option -pthread -fomit-frame-pointer > -ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden -g > -O0 -DDEBUG -D_DEBUG > -- C Compiler: > /home/jcmvbkbc/tmp/br/build-20160928-reproduce-opencv/host/usr/bin/xtensa-linux-gcc > -- C flags (Release): -D_LARGEFILE_SOURCE > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls > -mauto-litpools -Os -fsigned-char -W -Wall -Werror=return-type > -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point > -Wformat -Werror=format-security -Wmissing-declarations > -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self > -Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option > -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden > -O3 -DNDEBUG -DNDEBUG > -- C flags (Debug): -D_LARGEFILE_SOURCE > -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls > -mauto-litpools -Os -fsigned-char -W -Wall -Werror=return-type > -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point > -Wformat -Werror=format-security -Wmissing-declarations > -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self > -Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option > -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden > -g -O0 -DDEBUG -D_DEBUG > > Notice the difference between the C and the C++ flags. Indeed, there is a bug in the way we are currently setting/expending {C,CXX}FLAGS in the CMake toolchain file. I'll post a patch fixing it quickly. Regards,
Hello, since this patch has been included [1] in Samuel's series I'm marking it as superseded in patchwork. 1: https://patchwork.ozlabs.org/patch/682674/ Regards, Vincent. On 01/10/16 00:07, Samuel Martin wrote: > Hi all, > > On Fri, Sep 30, 2016 at 6:32 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: >> On Fri, Sep 30, 2016 at 1:10 AM, Thomas Petazzoni >> <thomas.petazzoni@free-electrons.com> wrote: >>> On Thu, 29 Sep 2016 09:14:41 -0700, Max Filippov wrote: >>>>> There is something fishy going on here. Indeed, >>>>> support/misc/toolchainfile.cmake.in contains: >>>>> >>>>> set(CMAKE_CXX_FLAGS "@@TARGET_CXXFLAGS@@ ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") >>>>> >>>>> So it should normally always prefix CMAKE_CXX_FLAGS with >>>>> TARGET_CXXFLAGS. Can you check the toolchain.cmake file that gets >>>>> generated during your build? Does it contains the Xtensa specific >>>>> CXXFLAGS ? >>>> >>>> Yes, it does. >>> >>> So if it does, why is your patch necessary? I'm not following here. >>> Do you mean that the Xtensa-specific flags are present in the >>> toolchain.cmake file, but are in the end not used during the build ? >> >> Right. The file >> host/usr/share/buildroot/toolchainfile.cmake >> exists and contains the following (correct) lines: >> >> set(CMAKE_C_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE >> -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os >> ${CMAKE_C_FLAGS}" CACHE STRING "Buildroot CFLAGS") >> set(CMAKE_CXX_FLAGS "-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE >> -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os >> ${CMAKE_CXX_FLAGS}" CACHE STRING "Buildroot CXXFLAGS") >> >> But the file >> build/opencv3-3.1.0/CMakeCache.txt >> that governs opencv3 build contains the following lines: >> >> //Buildroot CXXFLAGS >> CMAKE_CXX_FLAGS:STRING=-latomic >> //Buildroot CFLAGS >> CMAKE_C_FLAGS:STRING='-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE >> -D_FILE_OFFSET_BITS=64 -mlongcalls -mauto-litpools -Os ' >> >> Below is the output that I see during the failing opencv3 build: >> >> -- C++ flags (Release): -latomic -fsigned-char -W -Wall >> -Werror=return-type -Werror=non-virtual-dtor -Werror=address >> -Werror=sequence-point -Wformat -Werror=format-security >> -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow >> -Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor >> -fdiagnostics-show-option -pthread -fomit-frame-pointer >> -ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden >> -O3 -DNDEBUG -DNDEBUG >> -- C++ flags (Debug): -latomic -fsigned-char -W -Wall >> -Werror=return-type -Werror=non-virtual-dtor -Werror=address >> -Werror=sequence-point -Wformat -Werror=format-security >> -Wmissing-declarations -Wundef -Winit-self -Wpointer-arith -Wshadow >> -Wsign-promo -Wno-narrowing -Wno-delete-non-virtual-dtor >> -fdiagnostics-show-option -pthread -fomit-frame-pointer >> -ffunction-sections -fvisibility=hidden -fvisibility-inlines-hidden -g >> -O0 -DDEBUG -D_DEBUG >> -- C Compiler: >> /home/jcmvbkbc/tmp/br/build-20160928-reproduce-opencv/host/usr/bin/xtensa-linux-gcc >> -- C flags (Release): -D_LARGEFILE_SOURCE >> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls >> -mauto-litpools -Os -fsigned-char -W -Wall -Werror=return-type >> -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point >> -Wformat -Werror=format-security -Wmissing-declarations >> -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self >> -Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option >> -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden >> -O3 -DNDEBUG -DNDEBUG >> -- C flags (Debug): -D_LARGEFILE_SOURCE >> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -mlongcalls >> -mauto-litpools -Os -fsigned-char -W -Wall -Werror=return-type >> -Werror=non-virtual-dtor -Werror=address -Werror=sequence-point >> -Wformat -Werror=format-security -Wmissing-declarations >> -Wmissing-prototypes -Wstrict-prototypes -Wundef -Winit-self >> -Wpointer-arith -Wshadow -Wno-narrowing -fdiagnostics-show-option >> -pthread -fomit-frame-pointer -ffunction-sections -fvisibility=hidden >> -g -O0 -DDEBUG -D_DEBUG >> >> Notice the difference between the C and the C++ flags. > > Indeed, there is a bug in the way we are currently setting/expending > {C,CXX}FLAGS in the CMake toolchain file. > I'll post a patch fixing it quickly. > > Regards, >
diff --git a/package/opencv3/opencv3.mk b/package/opencv3/opencv3.mk index 2529de9..10660a9 100644 --- a/package/opencv3/opencv3.mk +++ b/package/opencv3/opencv3.mk @@ -12,7 +12,7 @@ OPENCV3_LICENSE_FILES = LICENSE # Uses __atomic_fetch_add_4 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y) -OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="-latomic" +OPENCV3_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic" endif # OpenCV component options
The commit 4904c4c "package/opencv3: use BR2_TOOLCHAIN_HAS_LIBATOMIC" overrides CMAKE_CXX_FLAGS with the single -latomic, losing all ABI CFLAGS that are passed there by default. This breaks build on xtensa where ABI CFLAGS contain important code generation options. Append $(TARGET_CXXFLAGS) to CMAKE_CXX_FLAGS along with -latomic. Fixes: http://autobuild.buildroot.net/results/7f1c96abd8fbb5b358a07100ab623316e9bb9dcd http://autobuild.buildroot.net/results/e0c93d0f6d1da0d62d4dbba211a275bfe75e9645 http://autobuild.buildroot.net/results/53e7e4b4b6a7b48b8012799d7507f7594dbf01b2 Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- package/opencv3/opencv3.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)