This is an archive of the discontinued LLVM Phabricator instance.

Differential D159156

[Support/BLAKE3] Fix error when building llvm for big endian AArch64 host
ClosedPublic

Authored by kovdan01 on Aug 29 2023, 7:44 PM.

Details

Reviewers
akyrtzi
jbms
foad
asl
Commits
rGea130a81d4c1: [Support/BLAKE3] Fix error when building llvm for big endian AArch64 host
Summary

BLAKE3 implementation does not support using arm neon on big-endian hosts: see
blake3_neon.c. Setting BLAKE3_USE_NEON to 1 by default for all AArch64
hosts broke builds for big endian hosts. This patch fixes the behavior
by introducing an additional check against __ARM_BIG_ENDIAN before
setting BLAKE3_USE_NEON.

Diff Detail

Event Timeline

kovdan01 created this revision.Aug 29 2023, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 7:44 PM
Herald added subscribers: StephenFan, hiraditya, kristof.beyls. · View Herald Transcript
kovdan01 requested review of this revision.Aug 29 2023, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2023, 7:44 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
asl accepted this revision.Aug 29 2023, 7:59 PM
asl added a subscriber: asl.

LGTM, thanks!

This revision is now accepted and ready to land.Aug 29 2023, 7:59 PM
Harbormaster completed remote builds in B255671: Diff 554550.Aug 29 2023, 10:38 PM
Closed by commit rGea130a81d4c1: [Support/BLAKE3] Fix error when building llvm for big endian AArch64 host (authored by kovdan01). · Explain WhyAug 30 2023, 1:47 AM
This revision was automatically updated to reflect the committed changes.
kovdan01 added a commit: rGea130a81d4c1: [Support/BLAKE3] Fix error when building llvm for big endian AArch64 host.
akyrtzi added a comment.Aug 30 2023, 9:55 AM

Could you bring up the issue with the BLAKE3 team as well? https://github.com/BLAKE3-team/BLAKE3

kovdan01 added a comment.Aug 30 2023, 4:17 PM
In D159156#4628872, @akyrtzi wrote:

Could you bring up the issue with the BLAKE3 team as well? https://github.com/BLAKE3-team/BLAKE3

Sure: submitted an issue https://github.com/BLAKE3-team/BLAKE3/issues/336 and a PR https://github.com/BLAKE3-team/BLAKE3/pull/335

MaskRay mentioned this in D138760: BLAKE3: do not try to use neon on big-endian aarch64.Sep 19 2023, 5:30 PM

Revision Contents

PathSize
llvm/
lib/
Support/
BLAKE3/
5 lines

Diff 554632

llvm/lib/Support/BLAKE3/blake3_impl.h

Show First 20 Lines • Show All 48 Lines • ▼ Show 20 Lines
#if defined(IS_X86) #if defined(IS_X86)
#if defined(_MSC_VER) #if defined(_MSC_VER)
#include <intrin.h> #include <intrin.h>
#endif #endif
#include <immintrin.h> #include <immintrin.h>
#endif #endif
#if !defined(BLAKE3_USE_NEON) #if !defined(BLAKE3_USE_NEON)
// If BLAKE3_USE_NEON not manually set, autodetect based on AArch64ness // If BLAKE3_USE_NEON not manually set, autodetect based on
#if defined(IS_AARCH64) // AArch64ness and endianness.
#if defined(IS_AARCH64) && !defined(__ARM_BIG_ENDIAN)
#define BLAKE3_USE_NEON 1 #define BLAKE3_USE_NEON 1
#else #else
#define BLAKE3_USE_NEON 0 #define BLAKE3_USE_NEON 0
#endif #endif
#endif #endif
#if defined(IS_X86) #if defined(IS_X86)
#define MAX_SIMD_DEGREE 16 #define MAX_SIMD_DEGREE 16
▲ Show 20 Lines • Show All 238 Lines • Show Last 20 Lines