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

WIP: MCST Elbrus initial port #37

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

a1batross
Copy link

@a1batross a1batross commented Apr 26, 2022

Needed some changes in crnlib and zstd.
Probably upgrading to upstream zstd or using a system provided library would be better

  • Compiles
  • Runs
  • Works

@a1batross
Copy link
Author

a1batross commented Apr 26, 2022

dimgui_impl.cpp file needs decoupling to separate files or just compiling imgui into a static library.

Overall, it just smells bad when code have #include <something.cpp> rather than compiling files one by one, potentially leveraging concurrency.

@feliwir
Copy link
Contributor

feliwir commented Apr 26, 2022

I'm currently in the process of replacing the insource dependencies - so the compilation should work with upstream versions of the libraries. See #15

@autious
Copy link
Collaborator

autious commented Apr 27, 2022

@feliwir does your upstreaming work collide with this pull request, or do you recommend that we pull this in?

@a1batross, i agree on the #include <f.cpp> thing.

@shinymerlyn
Copy link
Contributor

shinymerlyn commented Apr 27, 2022

Overall, it just smells bad when code have #include <something.cpp> rather than compiling files one by one, potentially leveraging concurrency.

This is not necessarily true. Linking time etc can dominate on some platforms. One of the reasons why "unity builds" are a thing.
But in this case the include wasn't done in order to improve performance, so I say do whatever way makes sense.

@feliwir
Copy link
Contributor

feliwir commented Apr 27, 2022

@autious
It does not collide with my work, but to me those libraries are thirdparty software - we should not modify it. Ideally we want to be able to just pull a new version of those libraries without breaking anything. However when we customize these libraries every update will be painful or we will just forget to readd the feature.

If something inside those libraries does not compile on any platform it should be fixed upstream and we can then upgrade to the version containing the fix.

E.g. for ZStd do a PR for that file inside the official repository: https://github.com/facebook/zstd/blob/v1.5.0/lib/common/compiler.h#L154

@Conan-Kudo Conan-Kudo added the blocked This issue or pull request cannot be resolved right now label Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue or pull request cannot be resolved right now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants