This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Define vector mask-register logical intrinsics.
ClosedPublic

Authored by khchen on Dec 22 2020, 7:18 AM.

Details

Summary

Define vector mask-register logical intrinsics and lower them
to V instructions. Also define pseudo instructions vmmv.m
and vmnot.m.

We work with @rogfer01 from BSC to come out this patch.

Authored-by: Roger Ferrer Ibanez <rofirrim@gmail.com>
Co-Authored-by: Zakk Chen <zakk.chen@sifive.com>

Diff Detail

Event Timeline

khchen created this revision.Dec 22 2020, 7:18 AM
khchen requested review of this revision.Dec 22 2020, 7:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2020, 7:18 AM
craig.topper added inline comments.Dec 22 2020, 9:59 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
198

This isn't used.

639

This should be AAA not AAX. The second argument is the same as the result.

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

Can you add the comments to these that appear in our downstream repo?

195

This line has M8 in our downstream repo.

llvm/test/CodeGen/RISCV/rvv/vmand-rv32.ll
4

We shouldn't need to say nxv1i1 twice. This will be fixed by using AAA instead of AAX in the Intrinsics file

74

Why does the test only go up to v8i1? Aren't there isel patterns for v16i1/v32i1/v64i1 as well?

craig.topper added inline comments.Dec 22 2020, 10:24 AM
llvm/include/llvm/IR/IntrinsicsRISCV.td
647

I don't think we need vmmv and vmnot. Those can be macros in the frontend.

khchen updated this revision to Diff 313489.Dec 22 2020, 11:08 PM
khchen marked an inline comment as done.
  1. remove vmmv and vmnot.
  2. add tests for v16i1/v32i1/v64i1
  3. add and use AAA intrinsic pattern
khchen marked 6 inline comments as done.EditedDec 22 2020, 11:09 PM

address @craig.topper 's comments.
thanks!

khchen updated this revision to Diff 313514.Dec 23 2020, 2:18 AM

remove vmmv and vmnot tests.

HsiangKai added inline comments.Dec 23 2020, 4:13 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
195

Is it reasonable to change the list as

defset list<MTypeInfo> AllMasks = {
  // vbool<n>_t, <n> = SEW/LMUL.
  def : MTypeInfo<vbool64_t, 8, V_MF8>;
  def : MTypeInfo<vbool32_t, 8, V_MF4>;
  def : MTypeInfo<vbool16_t, 8, V_MF2>;
  def : MTypeInfo<vbool8_t, 8, V_M1>;
  def : MTypeInfo<vbool4_t, 8, V_M2>;
  def : MTypeInfo<vbool2_t, 8, V_M4>;
  def : MTypeInfo<vbool1_t, 8, V_M8>;
}

If we could do in this way, SEW is redundant. We could set it to constant 8 in the patterns.

For mask logical operations, the important thing is the ratio of SEW/LMUL. If we could keep the relation of vbool<n>_t and the ratio correct, SEW should be not matter.

khchen added inline comments.Dec 23 2020, 8:09 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
195

Good point.

Should we still need to keep SEW filed in MTypeInfo (always equal to 8) for readability?

HsiangKai added inline comments.Dec 23 2020, 8:48 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
195

I think we could remove it and add comments in the pattern, i.e., /*SEW=*/8.

khchen updated this revision to Diff 313646.Dec 23 2020, 7:57 PM

address @HsiangKai's comments.

This revision is now accepted and ready to land.Dec 24 2020, 11:59 AM
This revision was automatically updated to reflect the committed changes.