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

E2k support #3288

Merged
merged 4 commits into from Aug 22, 2020
Merged

E2k support #3288

merged 4 commits into from Aug 22, 2020

Conversation

makise-homura
Copy link
Contributor

@makise-homura makise-homura commented Aug 18, 2020

Description

This pull request introduces support for building obs-studio on Elbrus hardware platform (which is based on Russian Elbrus CPU family) with its native lcc (eLbrus Compiler Collection) compiler.

Motivation and Context

As currently Elbrus-based stations are introduced into general puprose applications, including video streaming and recording, it is better to have OBS buildable on this platform directly from upstream, rather than being patched with some specific patches prior to building.

How Has This Been Tested?

Build and testing plan:

  • Build on Elbrus workstation (e2k-linux-gnu) and run there.
  • Build on x86_64 workstation (x86_64-linux-gnu) and run there (to be sure that nothing is broken on x86_64).

Details of Elbrus workstation:

  • CPU: Elbrus-8C (formerly Elbrus-8S) running on 1300 MHz
  • Motherboard: E8C-EATX
  • OS: OS Elbrus 5.0-rc2
  • GPU: Advanced Micro Devices, Inc. [AMD/ATI] Vega 10 XL/XT [Radeon RX Vega 56/64] (rev c1)
  • Kernel: 5.4.0-1.1-e8c
  • Compiler: lcc:1.24.09:Feb-11-2020:e2k-v4-linux (gcc (GCC) 7.3.0 compatible)

Details of x86_64 workstation:

  • CPU: AMD Ryzen Threadripper 2990WX 32-Core Processor running at 3.0 GHz
  • Motherboard: MSI X399 SLI PLUS (MS-7B09)
  • GPU: Advanced Micro Devices, Inc. [AMD/ATI] Caicos [Radeon HD 6450/7450/8450 / R5 230 OEM]
  • OS: Ubuntu 18.04.5 LTS
  • Kernel: 5.3.0-28-generic add new icon for os x bundle #30~18.04.1-Ubuntu
  • Compiler: gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

Results:

Successfully performed mkdir build; cd build; cmake -DCMAKE_BUILD_TYPE=Release .. && make -j16 && sudo make install and ran obs on both workstations; no suspicious behavior found.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@dodgepong
Copy link
Member

As currently Elbrus-based stations are introduced into general puprose applications, including video streaming and recording

Can you give examples of such stations? Where are they being deployed? What is the demand actually like, practically speaking? I have literally never even heard of Elbrus until this pull request, and I can't imagine the maintenance burden is going to be worth merging this.

@makise-homura
Copy link
Contributor Author

makise-homura commented Aug 18, 2020

Can you give examples of such stations? Where are they being deployed?

You may found some examples in MCST product catalog and INEUM product catalog. Unfortunately, these pages are in Russian yet, but you may use Google Translate if you want.
They are deployed in several organizations, like Bitblaze, Yandex, Rusbitech, etc. and even some enthusiasts (like Dmitry Bachilo) are having them at home or at work. Still the spread of Elbrus workstations is planned to be expanding.
Probably you didn't hear about it, because it's not well known outside of Russia, but some projects (e.g. Autotools, Meson, Taisei Project, inxi, DOSBox-X, etc., nearly full list (in Russian)) already have Elbrus support in upstream.

What is the demand actually like, practically speaking?

If you're speaking of numbers, there are several thousands of machines are deployed for now, and even more are planned to be made. Not as many as x86_64 or even ARM, or even RISC V, but at least not just single prototypes. If you're speaking of exact demand of OBS installed on these machines, I don't know exactly, but I've heard from some people that they want to have OBS on their workstations, but they can't just build it from the upstream due to issues that are being fixed by this PR.

I have literally never even heard of Elbrus until this pull request, and I can't imagine the maintenance burden is going to be worth merging this.

As this PR does not introduce major changes, but some clear, simple and obvious fixes, I see no reason it could lead to maintenance hell you're thinking of.

@WizardCM WizardCM added the Enhancement Improvement to existing functionality label Aug 19, 2020
@jp9000 jp9000 merged commit d9408b5 into obsproject:master Aug 22, 2020
@makise-homura makise-homura deleted the e2k_support branch August 22, 2020 17:58
@EntityFX
Copy link

Doom3 on e2k: https://youtu.be/6CGxTn_I3n0

@obsproject obsproject deleted a comment from SowingSadness Aug 24, 2020
@makise-homura makise-homura mentioned this pull request Jan 27, 2021
6 tasks
Comment on lines +138 to +142
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DSIMDE_ENABLE_OPENMP")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DSIMDE_ENABLE_OPENMP")
CHECK_C_COMPILER_FLAG("-fopenmp-simd" C_COMPILER_SUPPORTS_OPENMP_SIMD)
if(C_COMPILER_SUPPORTS_OPENMP_SIMD)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fopenmp-simd")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If openmp{,-simd} support is not enabled, then SIMDE_ENABLE_OPENMP should NOT be defined

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of lcc, openmp-simd is always enabled, rather than disabled (and -fopenmp-simd is just an unrecognized option).
For this omp.c:

int main() {
int j;
#pragma omp simd
for(int i=1;i<1000;++i) ++j;
}

That's what I get on x86_64:

flandre ~ # gcc -fopenmp-simd -Wunknown-pragmas omp.c
flandre ~ # gcc -Wunknown-pragmas omp.c
omp.c: In function ‘main’:
omp.c:3: warning: ignoring #pragma omp simd [-Wunknown-pragmas]
    3 | #pragma omp simd
      |
flandre ~ #

So, unless I specify -fopenmp-simd, there is no support for #pragma omp simd.
But that's what I get on Elbrus:

tenshi ~ # lcc -fopenmp-simd -Wunknown-pragmas omp.c
lcc: error: unrecognized command line option "-fopenmp-simd"
tenshi ~ # lcc -Wunknown-pragmas omp.c
tenshi ~ #

You see, #pragma omp simd is supported without any options, but once I specify -fopenmp-simd, it starts failing due to unrecognized option.

Probably I get your point: you're about these compilers, who might have no support for openmp-simd, and thus have no option (it's not the case of gcc, which has this option, and not the case of lcc either, which has it always enabled). It can be diverged by compiler identification, though; should I implement this case then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang doesn't have -fopenmp-simd, so that's the biggest problem.

The Intel compiler icc has a different, but equivalent option -qopenmp-simd that can be combined with -DSIMDE_ENABLE_OPENMP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok, I see. So you mean clang ends up in setting CMAKE_COMPILER_IS_GNUCC or CMAKE_COMPILER_IS_GNUCXX true, and therefore enters this block?
If so, then looks like you're right. Thus the correct way here seems to be like this: check_c_source_compiles() and check_cxx_source_compiles() of source with #pragma omp simd with -Werror-unknown-pragmas, and based on this, decide, if compiler which does not support -fopenmp-simd actually allows omp-simd or not. Correct?
If so, I'll make another PR for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I missed the CMAKE_COMPILER_IS_GNUCC part, whoops! So neither clang nor Intel's compiler will be affected by this section.

Seems that cmake doesn't know about the Elbrus lcc: https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER_ID.html#variable:CMAKE_%3CLANG%3E_COMPILER_ID

So this section is fine, but there should be a comment explaining that it is necessary due to Elbrus lcc being misidentified as a GNU compiler by cmake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the only thing needed to do here is place a comment describing a situation?

(Yes, CMake does not distinguish LCC from GCC; actually LCC tries to mimic GCC to be used in building in place of GCC despite not having 100% compatibility with it.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the only thing needed to do here is place a comment describing a situation?

Yep!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Should I post a PR for this then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Sorry for the confusion on my end.

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

Successfully merging this pull request may close these issues.

None yet

6 participants