Bug #2035

Core: Fix build of uppsrc/Core/Cpu.cpp for Clang compilers before 3.8.0 version

Added by Sender Ghost 4 months ago. Updated 4 months ago.

Status:ApprovedStart date:05/21/2020
Priority:HighDue date:
Assignee:Sender Ghost% Done:

0%

Category:CoreSpent time:-
Target version:-

Description

There is following error when using Clang 3.4.1 compiler on FreeBSD 10.4 amd64 (for Intel Core 2 Quad CPU):

In file included from <..>/upp/uppsrc/Core/Ops.h:347:
/usr/include/clang/3.4.1/smmintrin.h:28:2: error: "SSE4.1 instruction set not enabled" 
#error "SSE4.1 instruction set not enabled"

This is because of usage of

#ifndef __SSE4_1__
#error "SSE4.1 instruction set not enabled"

for smmintrin.h include before Clang 3.8.0 version:
https://github.com/llvm/llvm-project/blob/llvmorg-3.4.1/clang/lib/Headers/smmintrin.h#L27-L28
https://github.com/llvm/llvm-project/blob/llvmorg-3.7.1/clang/lib/Headers/smmintrin.h#L27-L28
https://github.com/llvm/llvm-project/blob/llvmorg-3.8.0/clang/lib/Headers/smmintrin.h#L26

The same source code builds ok with using Clang 9.0.1 compiler.

I propose to use emmintrin.h (SSE2) instead of smmintrin.h (SSE4.1) include and check for __SSE2__ define, because there is usage of SSE2 intrinsics in huge_memsetd and memsetd functions:
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm_set1_epi32&expand=4946

Some examples of how to use various *mmintrin.h header files:
uppsrc/plugin/Eigen/Eigen/src/Core/util/ConfigureVectorization.h:
https://github.com/ultimatepp/ultimatepp/blob/f963516253431f6cb8a7145ee6f3f8a5f61dd567/uppsrc/plugin/Eigen/Eigen/src/Core/util/ConfigureVectorization.h#L336-L359
uppsrc/plugin/glm/simd/platform.h:
https://github.com/ultimatepp/ultimatepp/blob/d10614cdcbacb498faae6685078a0fe463a5a4c9/uppsrc/plugin/glm/simd/platform.h#L325-L342

uppsrc_Core_r14486.diff Magnifier - Proposed patch for uppsrc (since 14486 revision) (839 Bytes) Sender Ghost, 05/21/2020 04:05 PM

uppsrc_Core_r14486_v2.diff Magnifier - Proposed patch for uppsrc (since 14486 revision, second variant) (2.64 KB) Sender Ghost, 05/21/2020 04:43 PM

uppsrc_Core_r14491.diff Magnifier - Proposed patch for uppsrc (since 14491 revision) (510 Bytes) Sender Ghost, 05/22/2020 12:17 PM

History

#1 Updated by Sender Ghost 4 months ago

  • Assignee set to Miroslav Fidler

#2 Updated by Sender Ghost 4 months ago

  • Description updated (diff)

#3 Updated by Sender Ghost 4 months ago

Attached second variant of the patch to fix reference/Eigen_demo build, mentioned on the forum:
https://www.ultimatepp.org/forums/index.php?t=msg&th=11026&goto=54009#msg_54009

#4 Updated by Iñaki Zabala 4 months ago

Please consider in addition that in Core/Oops.h, line 395, "RGBA" should have to be "dword" in inline void memsetd(void *p, RGBA c, int len)

#5 Updated by Sender Ghost 4 months ago

  • File uppsrc_Core_r14490.diff added

Looks like, the Eigen build issue with <smmintrin.h> include was because of its include inside of (Upp) namespace.

On the other hand, possible to use <immintrin.h> instead of <smmintrin.h> include, where <emmintrin.h> included conditionally, based on __SSE2__ define.

Patch attached.

#6 Updated by Sender Ghost 4 months ago

Patch updated after 14491 revision changes.

#7 Updated by Sender Ghost 4 months ago

  • File deleted (uppsrc_Core_r14490.diff)

#8 Updated by Sender Ghost 4 months ago

Possible to close, because it was fixed in 14492 revision.

#9 Updated by Iñaki Zabala 4 months ago

Tested fix OK in W10 (MSVC2019, CLANG, MinGW) and GNU Linux Ubuntu (CLANG, GCC).

#10 Updated by Sender Ghost 4 months ago

  • Status changed from Patch ready to Approved
  • Assignee changed from Miroslav Fidler to Sender Ghost

Iñaki Zabala wrote:

Tested fix OK in W10 (MSVC2019, CLANG, MinGW) and GNU Linux Ubuntu (CLANG, GCC).

Ok, thanks.

Also available in: Atom PDF