ilpincy / argos3

A parallel, multi-engine simulator for heterogeneous swarm robotics
http://www.argos-sim.info/
262 stars 121 forks source link

ARGoS not compiling with Bitbake/Yocto #168

Closed allsey87 closed 3 years ago

allsey87 commented 3 years ago

Having some difficultly compiling ARGoS on embedded at the moment, the flag -malign-double is being introduced somewhere which isn't compatible with the cross compiler:

arm-poky-linux-gnueabi-g++: error: unrecognized command line option '-malign-double'

It could be that it is just not supported on the ARM architecture, since I compiled the latest ARGoS for an Intel-based machine yesterday using Yocto without any problems.

allsey87 commented 3 years ago

This guilty commit is 84db11a. Citing the GCC documentation:

On x86-64, -malign-double is enabled by default.

So unless we care about performance on PCs more than 10 years old, there is no benefit to specifying this flag and a pretty serious downside to preventing compilation on ARM (the flag doesn't exist on ARM since it is illegal to align doubles on 4-byte boundaries).

ilpincy commented 3 years ago

Ok! Let's remove the flag then.

ilpincy commented 3 years ago

Can you give me some details on how you use this? On my robot I use a Target file that does not need to go through FindARGoS.

allsey87 commented 3 years ago

Do you mean cross compiling ARGoS via Yocto?

allsey87 commented 3 years ago

I think there might be some confusion since the commit that I linked to mentions "FindARGoS", however, -malign-double is being set from ARGoSBuildFlags.cmake, not from FindARGoS.cmake...

ilpincy commented 3 years ago

OK, I would suggest to remove that flag from the file altogether. I don't have the opportunity to test compilation with your environment - if you could remove the flag and test it, I'd be happy to make a new release this weekend to solve the issue.

allsey87 commented 3 years ago

Sounds good, I'll rebuild on my end and let you know :)

allsey87 commented 3 years ago

I removed -malign-double with following patch, but now have new issues with the compiler not being able to find basic header files like iostream and stdexcept. I will have to dig into this a bit further and find out what lines of the CMake a wreaking havoc on my build system.

From 60103cd279f1f31a7f1ba87cc4e257d2c599f7e5 Mon Sep 17 00:00:00 2001
From: Michael Allwright <allsey87@gmail.com>
Date: Fri, 9 Apr 2021 17:26:37 +0200
Subject: [PATCH] Remove -malign-double

---
 src/cmake/ARGoSBuildFlags.cmake | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/cmake/ARGoSBuildFlags.cmake b/src/cmake/ARGoSBuildFlags.cmake
index 11a4bf0d..ff401ea9 100644
--- a/src/cmake/ARGoSBuildFlags.cmake
+++ b/src/cmake/ARGoSBuildFlags.cmake
@@ -74,10 +74,6 @@ if(APPLE)
   set(CMAKE_MODULE_LINKER_FLAGS_DEBUG "-fsanitize=address")
 else(APPLE)
   # Linux
-  #
-  # Align doubles for higher performance
-  # Also: required by the PhysX engine
-  add_definitions(-malign-double)
   # Avoid discarding unused symbols to allow plugins to work
   set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-as-needed")
   set(ARGOS_SHARED_LIBRARY_EXTENSION "so")
-- 
2.25.1
ilpincy commented 3 years ago

I think that's on me. I decided to force the compilation flags in the cache, which (I now recognize) is a bad idea. Try to remove lines 88 onwards of ARGoSBuildFlags.cmake.

allsey87 commented 3 years ago

I have to head off for today, but I will give this a try on Monday and get back to you. Thanks for the heads up :)

allsey87 commented 3 years ago

I removed those lines. Same error messages regarding not being able to find iostream, stdexcept, and other standard headers. The last commit to compile successfully with Bitbake was c0ff091e9f037b022281cec6080c78fa3bd57879, 84db11a3f48c680dc4e76089c05bf58e2e048ce1 introduced both the problem with the -malign-double flag and the standard headers not being found.

allsey87 commented 3 years ago

The following patch against 84db11a3f48c680dc4e76089c05bf58e2e048ce1 fixes compilation. In addition to the malign-double issue, 84db11a3f48c680dc4e76089c05bf58e2e048ce1 reintroduces an issue that we solved in #62. I still need to do some more testing since some of the quality assurance tests built into Yocto are failing, which is a definite red flag to be investigated.

From 5f8e8c2523e4ee41e42e258c7bb84b931204f72c Mon Sep 17 00:00:00 2001
From: Michael Allwright <allsey87@gmail.com>
Date: Mon, 12 Apr 2021 11:59:16 +0200
Subject: [PATCH] Fix build flags

---
 src/cmake/ARGoSBuildFlags.cmake | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/cmake/ARGoSBuildFlags.cmake b/src/cmake/ARGoSBuildFlags.cmake
index 11a4bf0d..629363b6 100644
--- a/src/cmake/ARGoSBuildFlags.cmake
+++ b/src/cmake/ARGoSBuildFlags.cmake
@@ -23,20 +23,20 @@ set(CMAKE_CXX_EXTENSIONS OFF)
 #
 # General compilation flags
 #
-set(CMAKE_C_FLAGS "-Wall -Wno-unknown-pragmas")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wno-unknown-pragmas")
 if(ARGOS_BUILD_NATIVE)
   string(APPEND CMAKE_C_FLAGS "-mtune=native -march=native")
 endif(ARGOS_BUILD_NATIVE)
-set(CMAKE_C_FLAGS_RELEASE "-O3 -DNDEBUG")
-set(CMAKE_C_FLAGS_DEBUG "-ggdb3")
-set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_DEBUG} ${CMAKE_C_FLAGS_RELEASE}")
-set(CMAKE_CXX_FLAGS "-Wall -Wno-unknown-pragmas")
+set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -O3 -DNDEBUG")
+set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -ggdb3")
+set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${CMAKE_C_FLAGS_DEBUG} ${CMAKE_C_FLAGS_RELEASE}")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wno-unknown-pragmas")
 if(ARGOS_BUILD_NATIVE)
   string(APPEND CMAKE_CXX_FLAGS "-mtune=native -march=native")
 endif(ARGOS_BUILD_NATIVE)
-set(CMAKE_CXX_FLAGS_RELEASE        "-O3 -DNDEBUG")
-set(CMAKE_CXX_FLAGS_DEBUG          "-ggdb3")
-set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELEASE} ${CMAKE_CXX_FLAGS_DEBUG}")
+set(CMAKE_CXX_FLAGS_RELEASE        "${CMAKE_CXX_FLAGS_RELEASE} -O3 -DNDEBUG")
+set(CMAKE_CXX_FLAGS_DEBUG          "${CMAKE_CXX_FLAGS_DEBUG} -ggdb3")
+set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${CMAKE_CXX_FLAGS_RELEASE} ${CMAKE_CXX_FLAGS_DEBUG}")

 #
 # Reset linker flags
@@ -74,10 +74,6 @@ if(APPLE)
   set(CMAKE_MODULE_LINKER_FLAGS_DEBUG "-fsanitize=address")
 else(APPLE)
   # Linux
-  #
-  # Align doubles for higher performance
-  # Also: required by the PhysX engine
-  add_definitions(-malign-double)
   # Avoid discarding unused symbols to allow plugins to work
   set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-as-needed")
   set(ARGOS_SHARED_LIBRARY_EXTENSION "so")
-- 
2.25.1
allsey87 commented 3 years ago

I finally got everything to compile and link cleanly in my build system. The QA issues were due to the clobbering of linker flags and probably would not have executed properly on the robot's processor. My two final patches for resolving this issue are as follows, let me know how you would like to proceed, @ilpincy.

From 539788a5305ed6ada4d2a90c63f2b802250ea732 Mon Sep 17 00:00:00 2001
From: Michael Allwright <allsey87@gmail.com>
Date: Mon, 12 Apr 2021 14:43:50 +0200
Subject: [PATCH 1/2] Remove -malign-double flag

---
 src/cmake/ARGoSBuildFlags.cmake | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/cmake/ARGoSBuildFlags.cmake b/src/cmake/ARGoSBuildFlags.cmake
index 11a4bf0d..ff401ea9 100644
--- a/src/cmake/ARGoSBuildFlags.cmake
+++ b/src/cmake/ARGoSBuildFlags.cmake
@@ -74,10 +74,6 @@ if(APPLE)
   set(CMAKE_MODULE_LINKER_FLAGS_DEBUG "-fsanitize=address")
 else(APPLE)
   # Linux
-  #
-  # Align doubles for higher performance
-  # Also: required by the PhysX engine
-  add_definitions(-malign-double)
   # Avoid discarding unused symbols to allow plugins to work
   set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-as-needed")
   set(ARGOS_SHARED_LIBRARY_EXTENSION "so")
-- 
2.25.1
From c9500782fd3af9c394645ee3d7562f1a6032cd6f Mon Sep 17 00:00:00 2001
From: Michael Allwright <allsey87@gmail.com>
Date: Mon, 12 Apr 2021 14:49:42 +0200
Subject: [PATCH 2/2] Fix compiler and linker flags

---
 src/cmake/ARGoSBuildFlags.cmake | 39 ++++++++++-----------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/src/cmake/ARGoSBuildFlags.cmake b/src/cmake/ARGoSBuildFlags.cmake
index ff401ea9..764d9a08 100644
--- a/src/cmake/ARGoSBuildFlags.cmake
+++ b/src/cmake/ARGoSBuildFlags.cmake
@@ -23,36 +23,21 @@ set(CMAKE_CXX_EXTENSIONS OFF)
 #
 # General compilation flags
 #
-set(CMAKE_C_FLAGS "-Wall -Wno-unknown-pragmas")
+set(CMAKE_C_FLAGS                  "${CMAKE_C_FLAGS} -Wall -Wno-unknown-pragmas")
 if(ARGOS_BUILD_NATIVE)
-  string(APPEND CMAKE_C_FLAGS "-mtune=native -march=native")
+  set(CMAKE_C_FLAGS                "${CMAKE_C_FLAGS} -mtune=native -march=native")
 endif(ARGOS_BUILD_NATIVE)
-set(CMAKE_C_FLAGS_RELEASE "-O3 -DNDEBUG")
-set(CMAKE_C_FLAGS_DEBUG "-ggdb3")
-set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_DEBUG} ${CMAKE_C_FLAGS_RELEASE}")
-set(CMAKE_CXX_FLAGS "-Wall -Wno-unknown-pragmas")
+set(CMAKE_C_FLAGS_RELEASE          "${CMAKE_C_FLAGS_RELEASE} -O3 -DNDEBUG")
+set(CMAKE_C_FLAGS_DEBUG            "${CMAKE_C_FLAGS_DEBUG} -ggdb3")
+set(CMAKE_C_FLAGS_RELWITHDEBINFO   "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${CMAKE_C_FLAGS_DEBUG} ${CMAKE_C_FLAGS_RELEASE}")
+
+set(CMAKE_CXX_FLAGS                "${CMAKE_CXX_FLAGS} -Wall -Wno-unknown-pragmas")
 if(ARGOS_BUILD_NATIVE)
-  string(APPEND CMAKE_CXX_FLAGS "-mtune=native -march=native")
+  set(CMAKE_CXX_FLAGS              "${CMAKE_CXX_FLAGS} -mtune=native -march=native")
 endif(ARGOS_BUILD_NATIVE)
-set(CMAKE_CXX_FLAGS_RELEASE        "-O3 -DNDEBUG")
-set(CMAKE_CXX_FLAGS_DEBUG          "-ggdb3")
-set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELEASE} ${CMAKE_CXX_FLAGS_DEBUG}")
-
-#
-# Reset linker flags
-#
-unset(CMAKE_EXE_LINKER_FLAGS CACHE)
-unset(CMAKE_EXE_LINKER_FLAGS_DEBUG CACHE)
-unset(CMAKE_EXE_LINKER_FLAGS_RELEASE CACHE)
-unset(CMAKE_EXE_LINKER_FLAGS_RELWITHDEBINFO CACHE)
-unset(CMAKE_SHARED_LINKER_FLAGS CACHE)
-unset(CMAKE_SHARED_LINKER_FLAGS_DEBUG CACHE)
-unset(CMAKE_SHARED_LINKER_FLAGS_RELEASE CACHE)
-unset(CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO CACHE)
-unset(CMAKE_MODULE_LINKER_FLAGS CACHE)
-unset(CMAKE_MODULE_LINKER_FLAGS_DEBUG CACHE)
-unset(CMAKE_MODULE_LINKER_FLAGS_RELEASE CACHE)
-unset(CMAKE_MODULE_LINKER_FLAGS_RELWITHDEBINFO CACHE)
+set(CMAKE_CXX_FLAGS_RELEASE        "${CMAKE_CXX_FLAGS_RELEASE} -O3 -DNDEBUG")
+set(CMAKE_CXX_FLAGS_DEBUG          "${CMAKE_CXX_FLAGS_DEBUG} -ggdb3")
+set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${CMAKE_CXX_FLAGS_RELEASE} ${CMAKE_CXX_FLAGS_DEBUG}")

 if(APPLE)
   # MAC OSX
@@ -75,7 +60,7 @@ if(APPLE)
 else(APPLE)
   # Linux
   # Avoid discarding unused symbols to allow plugins to work
-  set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-as-needed")
+  set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--no-as-needed")
   set(ARGOS_SHARED_LIBRARY_EXTENSION "so")
   set(ARGOS_MODULE_LIBRARY_EXTENSION "so")
   set(ARGOS_DYNAMIC_LIBRARY_VARIABLE "LD_LIBRARY_PATH")
-- 
2.25.1
ilpincy commented 3 years ago

Thank you for fixing this! I'll incorporate your changes tomorrow. Sorry for the mess.

allsey87 commented 3 years ago

No problem, I will continue upgrading my repositories once this is incorporated and let you know if everything is working correctly.

allsey87 commented 3 years ago

Can confirm that 5c70ab2dd11415b80f83f6b94bdd4509e91e2c67 fixes compilation and packaging in Yocto. I have tested it for both an ARM and a Intel-based robot. I haven't launched ARGoS on the robots yet, but I suspect it will work fine.

ilpincy commented 3 years ago

Awesome, thanks for checking! :-)