[PATCH] D69791: [ARM,MVE] Add intrinsics for gather/scatter load/stores.

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham marked 3 inline comments as done.
simon_tatham added inline comments.


================
Comment at: clang/include/clang/Basic/arm_mve_defs.td:175
 
+// CopyKind<s,k> expects t and u to be scalars. It returns a scalar
+// whose kind (signed, unsigned or float) matches that of k, and whose
----------------
dmgreen wrote:
> Should the t and u be s and k?
Note to self: never change your mind about the variable names half way through writing a comment...


================
Comment at: clang/test/CodeGen/arm-mve-intrinsics/scatter-gather.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple thumbv8.1m.main-arm-none-eabi -target-feature +mve.fp -mfloat-abi hard -O0 -disable-O0-optnone -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+
----------------
dmgreen wrote:
> -DPOLYMORPHIC line
D'oh, that's what I get for copy-pasting the header from the one test file that //doesn't// have that extra line.


================
Comment at: llvm/include/llvm/IR/IntrinsicsARM.td:815
 
-def int_arm_mve_vcvt_narrow: Intrinsic<[llvm_v8f16_ty],
-   [llvm_v8f16_ty, llvm_v4f32_ty, llvm_i32_ty], [IntrNoMem]>;
-def int_arm_mve_vcvt_narrow_predicated: Intrinsic<[llvm_v8f16_ty],
-   [llvm_v8f16_ty, llvm_v4f32_ty, llvm_i32_ty, llvm_v4i1_ty], [IntrNoMem]>;
+multiclass MVEPredicated<list<LLVMType> rets, list<LLVMType> params,
+                         LLVMType pred, list<IntrinsicProperty> props = []> {
----------------
dmgreen wrote:
> This looks useful.
Eventually we'll probably need another one alongside it, for the many intrinsics that add an `inactive` parameter as well as a predicate mask. But for the moment I'm just adding what I need to use right now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69791/new/

https://reviews.llvm.org/D69791