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

[Clang] Compiling issues in Develop branch #102

Closed
simogasp opened this issue Mar 9, 2014 · 16 comments
Closed

[Clang] Compiling issues in Develop branch #102

simogasp opened this issue Mar 9, 2014 · 16 comments
Assignees
Milestone

Comments

@simogasp
Copy link
Contributor

simogasp commented Mar 9, 2014

Hi Pierre,

today I cloned a fresh copy of the repository and when I started compiling I got a lot of compilation errors. I saw there are some issues that have been solved in the develop branch so I switched to it. I still got a couple of errors though. Here's a brief report.

(Before starting, I'm on Mac OSX 10.9.2, compiling both with clang and gcc 4.8)

Issue with both clang and gcc

When linking openMVG_lInftyCV_test_resection_robust I got this error:

Linking CXX executable openMVG_lInftyCV_test_resection_robust
ld: warning: ignoring file ../libopenMVG_linearProgramming.a, file was built for archive which is not the architecture being linked (x86_64): ../libopenMVG_linearProgramming.a
ld: warning: ignoring file libopenMVG_lInftyComputerVision.a, file was built for archive which is not the architecture being linked (x86_64): libopenMVG_lInftyComputerVision.a
Undefined symbols for architecture x86_64:
  "openMVG::lInfinityCV::kernel::l1SixPointResectionSolver::Solve(Eigen::Matrix<double, -1, -1, 0, -1, -1> const&, Eigen::Matrix<double, -1, -1, 0, -1, -1> const&, std::__1::vector<Eigen::Matrix<double, 3, 4, 0, 3, 4>, std::__1::allocator<Eigen::Matrix<double, 3, 4, 0, 3, 4> > >*)", referenced from:
      openMVG::two_view::kernel::Kernel<openMVG::lInfinityCV::kernel::l1SixPointResectionSolver, openMVG::lInfinityCV::kernel::l1SixPointResectionSolver, Eigen::Matrix<double, 3, 4, 0, 3, 4> >::Fit(std::__1::vector<unsigned long, std::__1::allocator<unsigned long> > const&, std::__1::vector<Eigen::Matrix<double, 3, 4, 0, 3, 4>, std::__1::allocator<Eigen::Matrix<double, 3, 4, 0, 3, 4> > >*) const in resection_robust_test.cpp.o
ld: symbol(s) not found for architecture x86_64

After browsing the code a bit I found out that you have declared the l1SixPointResectionSolver kernel (in lInfinity/resection_kernel.hpp) without providing a method Fit(), as required by the function MaxConsensus() in robust_estimation/robust_estimator_MaxConsensus.hpp.
My solution was to... disable the test in the CMakeList.txt! 😄 Actually, I haven't had time to understand all the code, so I don't know exactly what goes inside the method Fit(). My suggestion, though, is to rather create an abstract class Kernel with the methods to implements and avoid using the template: in this case genericity has to be avoided as we explicitly call some class methods inside MaxConsensus().

Issue with clang

The other compilation error that I got only with clang occured when compiling SfMGlobalEngine. I got a bunch of this:

[ 97%] Building CXX object software/globalSfM/CMakeFiles/openMVG_main_GlobalSfM.dir/SfMGlobalEngine.cpp.o
In file included from /Users/sgaspari/dev/code/lib/openMVG/src/software/globalSfM/SfMGlobalEngine.cpp:23:
In file included from /Users/sgaspari/dev/code/lib/openMVG/src/./software/globalSfM/SfMGlobal_tij_computation.hpp:22:
/Users/sgaspari/dev/code/lib/openMVG/src/./software/globalSfM/mutexSet.hpp:29:7: error: reference to 'lock_guard' is ambiguous
      lock_guard<mutexT> guard(m_Mutex);
      ^
/Users/sgaspari/dev/code/lib/openMVG/src/./third_party/tinythread/tinythread.h:343:7: note: candidate found by name lookup is 'tthread::lock_guard'
class lock_guard {
      ^

So I guess clang (for reference, my version is 500.2.79) is a bit more conservative when resolving ambiguous references. I added tthread:: as suggested wherever the compiler complained and everything went smooth.

In case I can make you a pull request for this minor changes.

A+

S.

pmoulon added a commit that referenced this issue Mar 9, 2014
@pmoulon
Copy link
Member

pmoulon commented Mar 9, 2014

Hi Simone,
Thanks for the detailed reporting.

I have tried on clang and gcc two different linux and everything run fine (fine for the travis CI too).
It seems that on Max OS X compilers are less permissive...

For openMVG_lInftyCV_test_resection_robust:
It must compile since lInfinityCV::kernel::l1PoseResectionKernel implements the requested Fit method.

For the tthread issue I have tried to pull something that will be ok for CXX11 support for later version.

Can you tell me if it is ok on your side?

Thanks @simogasp

@simogasp
Copy link
Contributor Author

simogasp commented Mar 9, 2014

Hi Pierre,
I was dealing with this kind of issues all week long, so I waited to submit the issue because I thought it may be something wrong just on my side. I can confirm that everything compiles fines for me on any linux distribution (Linux Mint on virtualbox and Ubuntu on a real machine) but I still have the kernel problem with the above compilation error.

Beside the compilation error, I'm referring to this
https://github.com/openMVG/openMVG/blob/develop/src/openMVG/linearProgramming/lInfinityCV/resection_kernel.hpp#L27

I don't see any Fit() method. On the other hand I agree it is kind of bizarre that it compiles on the other systems (but maybe it will give a run-time error in the test, I haven't tried that). Maybe some compilers are more permissive with the type checking, I don't know...

As for the other error, if I comment out the previous test case, everything compiles smoothly.

Thanks!

S.

PS
I also tried cloning a fresh copy of the develop branch, same result.

@pmoulon
Copy link
Member

pmoulon commented Mar 10, 2014

Hi Simone,
All robust estimation method use the kernel concept.

Kernel concept embed reference to the data used for the fitting and a model solver and a metric (to measure data consistency to the computed model).

The object you referring too is the model solver l1SixPointResectionSolver that is used just below in a KERNEL l1PoseResectionKernel.
=> See here for the Fit method that call the solve method from the model solver.
https://github.com/openMVG/openMVG/blob/master/src/openMVG/multiview/two_view_kernel.hpp

We need to understand the strange behavior you have met.

@simogasp
Copy link
Contributor Author

Yes, sorry, I got lost in the code and I didn't see well what you did in the typedef.
It's really bizarre, I'll keep on looking into that and let you know.

S.

@simogasp
Copy link
Contributor Author

Hi Pierre,
today I accidentally let the system to install a new update for Xcode which contains a new version of clang, Apple LLVM version 5.1 (clang-503.0.38).
So the bad news is that now I got also some new compilation errors for ceres... I report them here below. Once those are fixed, I'm back to square one, even with the new version I have the same error above...

Here are a bunch of things to fix in ceres:

  • The first compilation error occurred with the very first file:
[ 33%] Building CXX object third_party/ceres-solver/internal/ceres/CMakeFiles/miniglog.dir/miniglog/glog/logging.cc.o
clang: error: -O4 is equivalent to -O3

Apparently in the new version of clang -O4 no longer implies link time optimization (LTO) (see here for details, Compiler section). I'm not an expert but everything seems to compile fine by changing the line 510 of third_party/ceres-solver/CMakeLists.txt into
SET(CERES_CXX_FLAGS "${CERES_CXX_FLAGS} -flto") which apparently is the "new" way to impose the LTO.

  • There is another compilation error which is actually just a warning, but as far I understand google guys do not like warnings when compiling.... 😉
[ 68%] Building CXX object dependencies/osi_clp/Clp/src/CMakeFiles/lib_clp.dir/ClpGubDynamicMatrix.cpp.o
/Users/sgaspari/dev/code/lib/openMVG/src/third_party/ceres-solver/internal/ceres/conjugate_gradients_solver.cc:59:14: error: unused variable 'kEpsilon' [-Werror,-Wunused-const-variable]
const double kEpsilon = 2.2204e-16;
             ^
1 error generated.

Commenting out the constant everything goes fine.

A+

S.

@pmoulon pmoulon changed the title Compiling issues in Develop branch [Clang] Compiling issues in Develop branch Apr 2, 2014
@pmoulon
Copy link
Member

pmoulon commented May 28, 2014

@simogasp can you tell me if the 0.6 version is ok ?
Just checkout the last master revision

@pmoulon
Copy link
Member

pmoulon commented Jun 1, 2014

Compilation with clang is now checked on travis.

@pmoulon pmoulon closed this as completed Jun 1, 2014
@simogasp
Copy link
Contributor Author

simogasp commented Jun 1, 2014

I still have problems compiling with clang (LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)):

  • one issue is still from Ceres with the -O4 option (see above)
  • then, when I arrive at 100% linking all the executable I have that issue I mentioned to you weeks ago about the assembler code (??), eg.
Linking CXX executable openMVG_main_IncrementalSfM
clang: error: unable to execute command: Illegal instruction: 4
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [software/globalSfM/openMVG_main_GlobalSfM] Error 254
make[1]: *** [software/globalSfM/CMakeFiles/openMVG_main_GlobalSfM.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
Instruction does not dominate all uses!
  %147 = getelementptr inbounds double* %A, i64 %146
  %bound168 = icmp ule double* %147, %scevgep52
Instruction does not dominate all uses!
  %147 = getelementptr inbounds double* %A, i64 %146
  %bound067 = icmp ule double* %scevgep, %147
Broken module found, compilation aborted!

which may be a bug in clang (http://llvm.org/bugs/show_bug.cgi?id=18768) or maybe something on my side.
Otherwise gcc on mac works.

S.

@simogasp
Copy link
Contributor Author

simogasp commented Jun 1, 2014

BTW which clang version are u using on travis? is there a way to find it out?

@donlk
Copy link
Contributor

donlk commented Jun 1, 2014

Not that it is the source of the problem, but why compile with -O4 flag?
Anything above O3 resolves to O3 anyway, you dont get any benefits from
them.
On Jun 1, 2014 2:59 PM, "Simone Gasparini" notifications@github.com wrote:

I still have problems compiling with clang (LLVM version 5.1
(clang-503.0.40) (based on LLVM 3.4svn)):

  • one issue is still from Ceres with the -O4 option (see above)
  • then, when I arrive at 100% linking all the executable I have that
    issue I mentioned to you weeks ago about the assembler code (??), eg.

Linking CXX executable openMVG_main_IncrementalSfM
clang: error: unable to execute command: Illegal instruction: 4
clang: error: linker command failed due to signal (use -v to see invocation)
make[2]: *** [software/globalSfM/openMVG_main_GlobalSfM] Error 254
make[1]: *** [software/globalSfM/CMakeFiles/openMVG_main_GlobalSfM.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
Instruction does not dominate all uses!
%147 = getelementptr inbounds double* %A, i64 %146
%bound168 = icmp ule double* %147, %scevgep52
Instruction does not dominate all uses!
%147 = getelementptr inbounds double* %A, i64 %146
%bound067 = icmp ule double* %scevgep, %147
Broken module found, compilation aborted!

which may be a bug in clang (http://llvm.org/bugs/show_bug.cgi?id=18768)
or maybe something on my side.
Otherwise gcc on mac works.

S.


Reply to this email directly or view it on GitHub
#102 (comment).

@pmoulon
Copy link
Member

pmoulon commented Jun 1, 2014

@simogasp : I have update the last version of Ceres in the develop branch.
Perhaps they have fixed the compilation problem on their side.

For the assembler problem I don't know where it comes from.

@pmoulon pmoulon reopened this Jun 1, 2014
@simogasp
Copy link
Contributor Author

simogasp commented Jun 1, 2014

@donlk see the link in my message above, it's something related to the new version of clang. It's set by default by Ceres, so it should be up to them to fix it. With my workaround described above it works for me.

@pmoulon i checked and even in the last version of ceres it is not fixed. As for the assembler error I have no clue. I will try to install an older version of clang (and next time i'll be more careful when accepting updates :-)

@pmoulon pmoulon added this to the Putative 0.7 milestone Jun 30, 2014
@pmoulon pmoulon self-assigned this Jun 30, 2014
@simogasp
Copy link
Contributor Author

simogasp commented Jul 2, 2014

Hi @pmoulon ,
just an update: following your suggestion, I suppressed the -flto option in Ceres' CMakeLists and now everything compiles smoothly! I can compile the latest develop branch without any issue.
I don't know if this affects only the latest version of the clang, anyway from the comments in the CMakeLists it may be the case that the CHECK_CXX_COMPILER_FLAG() macro does not work properly with the -flto option (or, better, clang does not complain in the expected way when trying to compile with that flag without the gold plugin).
However, we can maybe add a remainder/note in the README to disable the option in case the Broken module error is shown.

Thanks!

S.

@pmoulon
Copy link
Member

pmoulon commented Jul 2, 2014

@rperrot Your guess was right.
Perhaps the fast workaround could be to disable the -flto option if in cmake we detect clang and MacOs architecture. (As FLTO must just make the linking step faster it's not a big deal).

@rperrot
Copy link
Contributor

rperrot commented Jul 2, 2014

@pmoulon To be a little bit more precise :

  • we need to know if the only official (ie: provided by Apple) version of clang produces the error (what about macports/homebrew/fink/... versions ?)
  • we need to know if it's a bug related to clang on OSX or on all version of clang (linux/windows/...) ;

But for a quick fix, disabling flto on all clang builds could be fine since it does not have any impact on openMVG performance.

@pmoulon
Copy link
Member

pmoulon commented Jul 4, 2014

@simogasp @rperrot Now the link compilation error must disappear on OSX clang (branch develop).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants