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

The embedded OpenGL loader prevents LTO compilation #6668

Closed
nicolasnoble opened this issue Jul 31, 2023 · 3 comments
Closed

The embedded OpenGL loader prevents LTO compilation #6668

nicolasnoble opened this issue Jul 31, 2023 · 3 comments

Comments

@nicolasnoble
Copy link
Contributor

Version/Branch of Dear ImGui:

Version: HEAD
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_opengl3.cpp
Compiler: gcc using -flto
Operating System: Linux, but shouldn't really matter

My Issue/Question:
So, some time ago, ImGui decided (with some good reasons) to have a private OpenGL loader which is basically a mutated gl3w, which is fine at face value.

The problem I am facing however now is that my own application is also using gl3w, which is also fine at face value, but I decided to give LTO a go. The LTO compilation fails (and falls back to serial compilation) due to the following:

third_party/gl3w/GL/gl3w.h:56:16: warning: type ‘struct <anon>’ violates the C++ One Definition Rule [-Wodr]
   56 |         struct {
      |                ^
third_party/imgui/backends/imgui_impl_opengl3_loader.h:472: note: a different type is defined in another translation unit
  472 |     struct {
      | 
third_party/gl3w/GL/gl3w.h:57:73: note: the first difference of corresponding definitions is field ‘ActiveProgramEXT’
   57 |                 PFNGLACTIVEPROGRAMEXTPROC                               ActiveProgramEXT;
      |                                                                         ^
third_party/imgui/backends/imgui_impl_opengl3_loader.h:473: note: a field with different name is defined in another translation unit
  473 |         PFNGLACTIVETEXTUREPROC            ActiveTexture;
      | 
third_party/gl3w/GL/gl3w.h:54:7: warning: type ‘union GL3WProcs’ violates the C++ One Definition Rule [-Wodr]
   54 | union GL3WProcs {
      |       ^
third_party/imgui/backends/imgui_impl_opengl3_loader.h:470: note: a different type is defined in another translation unit
  470 | union GL3WProcs {
      | 
third_party/gl3w/GL/gl3w.h:55:20: note: the first difference of corresponding definitions is field ‘ptr’
   55 |         GL3WglProc ptr[1278];
      |                    ^
third_party/imgui/backends/imgui_impl_opengl3_loader.h:471: note: a field of same name but different type is defined in another translation unit
  471 |     GL3WglProc ptr[59];
      | 
third_party/gl3w/GL/gl3w.h:54:7: note: array types have different bounds
   54 | union GL3WProcs {
      |       ^

Basically, the LTO linker is complaining that the same union has been duplicated throughout the whole of the monolithic compilation unit, and can't proceed properly, and so fall backs to doing serial compilation, nullifying the LTO effort there.

Suggestion to alleviate this: the modified gl3w generator could simply rename the GL3WProcs union to ImGL3WProcs, the same way it already has renamed imgl3wProcs, which should then properly enable LTO builds.

@ocornut
Copy link
Owner

ocornut commented Aug 1, 2023

Suggestion to alleviate this: the modified gl3w generator could simply rename the GL3WProcs union to ImGL3WProcs, the same way it already has renamed imgl3wProcs, which should then properly enable LTO builds.

Yes, I'd be okay to do this. Since if you have a way to test the output and find other cases I'd appreciate if you can make the PR for it in gl3w_stripped repo.

@nicolasnoble
Copy link
Contributor Author

Yes, I'd be okay to do this. Since if you have a way to test the output and find other cases I'd appreciate if you can make the PR for it in gl3w_stripped repo.

Sounds good, I'll go ahead with testing locally and issue PRs.

@ocornut
Copy link
Owner

ocornut commented Oct 5, 2023

Merged 64b1aee, thank you !

@ocornut ocornut closed this as completed Oct 5, 2023
ocornut pushed a commit that referenced this issue Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants