This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add undisturbed version of unmasked intrinsics.
AbandonedPublic

Authored by HsiangKai on Sep 30 2021, 6:43 AM.

Details

Summary

In https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/116, we
propose a new set of intrinsics for unmasked tail undisturbed
instructions. This patch is the implementation of the proposal.

This is a preparation work for these new C intrinsics. This patch only
adds LLVM IR intrinsics. There is no compile time or binary size
problem.

Diff Detail

Event Timeline

HsiangKai created this revision.Sep 30 2021, 6:43 AM
HsiangKai requested review of this revision.Sep 30 2021, 6:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2021, 6:43 AM
HsiangKai retitled this revision from [PoC][RISCV] Add undisturbed version of unmasked intrinsics. Draft version. Proof of concept of https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/116. to [PoC][RISCV] Add undisturbed version of unmasked intrinsics..Sep 30 2021, 6:44 AM
HsiangKai edited the summary of this revision. (Show Details)

Full implementation of LLVM IR intrinsics for unmasked tail undisturbed intrinsics.

HsiangKai retitled this revision from [PoC][RISCV] Add undisturbed version of unmasked intrinsics. to [RISCV] Add undisturbed version of unmasked intrinsics..Oct 18 2021, 11:44 PM
HsiangKai edited the summary of this revision. (Show Details)

Fix build errors.

HsiangKai updated this revision to Diff 380639.Oct 19 2021, 4:20 AM

Add test cases.

Handle vmv.v.x, vfmv.v.f, and vector loads.

khchen added a subscriber: khchen.Nov 8 2021, 7:45 AM
khchen added inline comments.
llvm/include/llvm/IR/IntrinsicsRISCV.td
203

// Input: (undisturbed, pointer, vl) maybe better.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
479

How about

class RISCVVSX<bit M, bit O, bits<3> S, bits<3> L, bits<3> IL> :
  RISCVVLX_VSX<M, /*TU*/ 0, O, S, L, IL>;
HsiangKai updated this revision to Diff 394462.Dec 14 2021, 8:12 PM
  • Fix build errors.
  • Deal with 'tied' pseudo instructions.

Address @khchen's comments.

HsiangKai marked 2 inline comments as done.Dec 15 2021, 10:05 PM

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)?

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?

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.

llvm/include/llvm/IR/IntrinsicsRISCV.td
1036

Do we need have a note to tell why _tu and non _tu version inherit the same class?

1221

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?

1248

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?

khchen requested changes to this revision.Dec 28 2021, 7:59 PM
khchen added inline comments.
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
1755

you need to add parentheses to update VLMul for _TU version pseduoInst.

This revision now requires changes to proceed.Dec 28 2021, 7:59 PM
HsiangKai abandoned this revision.Dec 22 2022, 1:03 AM

Out of date.