clangir icon indicating copy to clipboard operation
clangir copied to clipboard

AArch64 specific builtins/intrinsics

Open bcardosolopes opened this issue 9 months ago • 6 comments

We currently don't emit ARM64 specific intrinsics/builtins, nor none for other arches as well. See clang/lib/CIR/CodeGen/CIRGenBuiltinAArch64.cpp for the paths full of asserts. The suggested way to tackle these are by incrementally adding support to the missing pieces - there's a huge number of them.

bcardosolopes avatar May 06 '24 17:05 bcardosolopes

Hey, is this issue still open?

khoipkv avatar Jun 03 '24 17:06 khoipkv

Yep, and there's enough work for multiple folks!

bcardosolopes avatar Jun 03 '24 22:06 bcardosolopes

See commented tests in clang/test/CIR/CodeGen/aarch64-neon-intrinsics.c. This is just a subset, there is even more if you search in clang/test/CodeGen.

bcardosolopes avatar Jun 03 '24 22:06 bcardosolopes

If so, could you please assign this one to me? If anyone wants to join in, we add more.

khoipkv avatar Jun 04 '24 14:06 khoipkv

@khoipkv Here's a list of ones I'd give priority:

Start with:
NEON::BI__builtin_neon_vld1_lane_v
NEON::BI__builtin_neon_vld1q_lane_v
NEON::BI__builtin_neon_vst1_lane_v
NEON::BI__builtin_neon_vst1q_lane_v
NEON::BI__builtin_neon_vld1_dup_v
NEON::BI__builtin_neon_vld1q_dup_v

Next ones:
NEON::BI__builtin_neon_vsetq_lane_f64
NEON::BI__builtin_neon_vdups_lane_i32
NEON::BI__builtin_neon_vrshrn_n_v
NEON::BI__builtin_neon_vget_lane_i8
NEON::BI__builtin_neon_vdupb_lane_i8
clang::AArch64::BI__builtin_arm_ldrex
clang::AArch64::BI__builtin_arm_ldaex
NEON::BI__builtin_neon_vqrshrun_n_v
NEON::BI__builtin_neon_vsra_n_v
NEON::BI__builtin_neon_vsraq_n_v
NEON::BI__builtin_neon_vget_lane_i64
NEON::BI__builtin_neon_vdupd_lane_i64
NEON::BI__builtin_neon_vgetq_lane_i64
NEON::BI__builtin_neon_vdupd_laneq_i64

I suggest you try to get one PR for each individually (or few relevant ones together), incremental is best to make progress and ramp up.

After you complete this list, you might gain a bit more familiarity with how builtins work. Then, you could go for the other bulk ones (table driven), like in clang/lib/CIR/CodeGen/CIRGenBuiltinAArch64.cpp:2242:

  // Not all intrinsics handled by the common case work for AArch64 yet, so only
  // defer to common code if it's been added to our special map.
  Builtin = findARMVectorIntrinsicInMap(AArch64SIMDIntrinsicMap, BuiltinID,
                                        AArch64SIMDIntrinsicsProvenSorted);
  if (Builtin) {
    llvm_unreachable("NYI");
  }

(cc: @dkolsen-pgi)

bcardosolopes avatar Jul 08 '24 22:07 bcardosolopes

Is there work still left to be done on this?

NoumanAmir657 avatar Aug 31 '24 09:08 NoumanAmir657