This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][Clang] Add bf16-type vector support for RVV
Needs ReviewPublic

Authored by joshua-arch1 on Jun 8 2023, 10:38 PM.

Details

Summary

New RVV types using bfloat16 like vbfloat16m1_t are being introduced (https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/229). Now we need to provide clang support in order for RVV intrinsics using bfloat16 to work..

Diff Detail

Event Timeline

joshua-arch1 created this revision.Jun 8 2023, 10:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 8 2023, 10:38 PM
joshua-arch1 requested review of this revision.Jun 8 2023, 10:38 PM

No tests showing IR output?

joshua-arch1 edited the summary of this revision. (Show Details)EditedJun 8 2023, 10:42 PM

No tests showing IR output?

No specific bf16 intrinsics have been landed right now. This patch in only to add bf16 type to RVV.

craig.topper added a comment.EditedJun 8 2023, 11:14 PM

No tests showing IR output?

No specific bf16 intrinsics have been landed right now. This patch in only to add bf16 type to RVV.

Adding the type means it can be loaded, stored, returned, and passed to functions. Those all need to be tested and must not crash the backend.

  1. Add argument IsBF to RVV_VECTOR_TYPE()
  2. Add test for bf16 vectors in rvv-intrinsic-datatypes.cpp

I don’t see tests for load or store or function arguments or returns.

Do the IR types need to be made legal in the backend.

clang/utils/TableGen/RISCVVEmitter.cpp
383

Does this line exceed 80 characters?

I think we need to check the extension in Sema::checkTypeSupport. See the existing code there

// RISC-V vector builtin types (RISCVVTypes.def)                             
if (Ty->isRVVType(/* Bitwidth */ 64, /* IsFloat */ false) &&                 
    !TI.hasFeature("zve64x"))                                                
  Diag(Loc, diag::err_riscv_type_requires_extension, FD) << Ty << "zve64x";  
if (Ty->isRVVType(/* Bitwidth */ 16, /* IsFloat */ true) &&                  
    !TI.hasFeature("experimental-zvfh"))                                     
  Diag(Loc, diag::err_riscv_type_requires_extension, FD)                     
      << Ty << "zvfh";                                                       
if (Ty->isRVVType(/* Bitwidth */ 32, /* IsFloat */ true) &&                  
    !TI.hasFeature("zve32f"))                                                
  Diag(Loc, diag::err_riscv_type_requires_extension, FD) << Ty << "zve32f";  
if (Ty->isRVVType(/* Bitwidth */ 64, /* IsFloat */ true) &&                  
    !TI.hasFeature("zve64d"))                                                
  Diag(Loc, diag::err_riscv_type_requires_extension, FD) << Ty << "zve64d";

I don’t see tests for load or store or function arguments or returns.

Do the IR types need to be made legal in the backend.

Right now we do not have rvv api with bfloat16 type. It will be difficult to create load/store functions using bfloat16-type vectors. Maybe we can wait until these intrinsics get ratified (https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/229/files).

I don’t see tests for load or store or function arguments or returns.

Do the IR types need to be made legal in the backend.

Right now we do not have rvv api with bfloat16 type. It will be difficult to create load/store functions using bfloat16-type vectors. Maybe we can wait until these intrinsics get ratified (https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/229/files).

It’s not hard to create a load or store. You just need a vbfloat16m1_t pointer and a dereference.

I don’t see tests for load or store or function arguments or returns.

Do the IR types need to be made legal in the backend.

Right now we do not have rvv api with bfloat16 type. It will be difficult to create load/store functions using bfloat16-type vectors. Maybe we can wait until these intrinsics get ratified (https://github.com/riscv-non-isa/rvv-intrinsic-doc/pull/229/files).

It’s not hard to create a load or store. You just need a vbfloat16m1_t pointer and a dereference.

-O0 probably creates loads and stores without really doing much.

sunshaoce added inline comments.Sep 25 2023, 3:54 AM
clang/include/clang/Basic/riscv_vector_common.td
44
eopXD added inline comments.Sep 25 2023, 4:34 AM
clang/include/clang/Basic/riscv_vector_common.td
44

Yes indeed, @joshua-arch1 do you mind reopening a pull request in Github? If not, I will be willing to pick up the rest of the work.

Yes indeed, @joshua-arch1 do you mind reopening a pull request in Github? If not, I will be willing to pick up the rest of the work.

It has been a long time, and bf16 is quite important. @eopXD, can you continue the development? And then add @joshua-arch1 the author?