[PATCH] D110831: [RISCV] Add undisturbed version of unmasked intrinsics.

[Zakk Chen via Phabricator via llvm-commits]

khchen added a comment.

In D110831#3202244 <https://reviews.llvm.org/D110831#3202244>, @HsiangKai wrote:

> In D110831#3197843 <https://reviews.llvm.org/D110831#3197843>, @khchen wrote:
>
>> LGTM. I would like to wait for others reviewer's comments.
>> BTW, why the default tail undisturbed version of unmasked intrinsics is using `mu` (mask undisturbed)?
>
> There seems no flag to distinguish masked and unmasked pseudo instructions. The default value of masked pseudo instructions is `mu`. How about to leave it as another patch to annotate the `mask` information for pseudo instructions and change the default value according to the flag?

Okay, It makes sense to me.



================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:1036
     def "int_riscv_" # NAME : RISCVTernaryAAAXNoMask;
+    def "int_riscv_" # NAME # "_tu" : RISCVTernaryAAAXNoMask;
     def "int_riscv_" # NAME # "_mask" : RISCVTernaryAAAXMask;
----------------
Do we need have a note to tell why `_tu` and non `_tu` version inherit the same class?



================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:1221
 
   defm vmerge : RISCVBinaryWithV0;
 
----------------
Spec said `vmerge` operates on all body elements from vstart up to the current vector length in vl,  so it seems like vmerge/vfmerge have `_tu` version intrinsics?


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:1248
                                     [IntrNoMem]>, RISCVVIntrinsic;
   def int_riscv_vmv_s_x : Intrinsic<[llvm_anyint_ty],
                                     [LLVMMatchType<0>, LLVMVectorElementType<0>,
----------------
we have `_tu` suffix now and default `int_riscv_vmv_s_x` is tail undisturbed, do we need to rename it as int_riscv_vmv_s_x_tu?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110831