Page MenuHomePhabricator

[Clang][RISCV] Implement vlseg and vlsegff.
ClosedPublic

Authored by HsiangKai on Jun 2 2021, 8:09 AM.

Diff Detail

Event Timeline

HsiangKai created this revision.Jun 2 2021, 8:09 AM
HsiangKai requested review of this revision.Jun 2 2021, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2021, 8:09 AM
craig.topper added inline comments.Jun 2 2021, 1:55 PM
clang/include/clang/Basic/riscv_vector.td
807

This can be a llvm::Value Operands[] = I think?

877

This can be a llvm::Value Operands[] = I think?

clang/utils/TableGen/RISCVVEmitter.cpp
441

Point->Pointer?

1120

Put curly braces around these if and else bodies since they contain a comment. The compiler doesn't need them but its more readable for humans and would be consistent with the standards documented here https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

HsiangKai updated this revision to Diff 349754.Jun 3 2021, 8:09 PM

Address comments.

HsiangKai marked 4 inline comments as done.Jun 3 2021, 8:10 PM

Do you think you can split vlseg and vlseg_ff tests in two different files? So we (potentially) reduce the test latency by half in multicore systems.

Other than this, LGTM.

We're a bit split in https://github.com/riscv/rvv-intrinsic-doc/issues/95 However there are mechanisms to assist in any transition should we be able to implement in the future alternative interfaces that we prefer over these ones.

HsiangKai updated this revision to Diff 354117.Jun 23 2021, 5:22 PM

Split vlseg and vlseg_ff tests in two different files.

Ping. Is it possible to land these Zvlsseg patches before LLVM 13 branch created?

This revision is now accepted and ready to land.Tue, Jul 6, 12:12 PM
This revision was automatically updated to reflect the committed changes.
craig.topper added inline comments.Mon, Jul 12, 6:51 PM
clang/include/clang/Basic/riscv_vector.td
836

I don't think this alignment is correct. A vint16mf4_t creates an alloca with align of 2 and vint8mf4_t creates an alloca with an align of 1. So I think the store here needs to match the alignment you would get for the type we're storing. This is an issue in the earlier vlseg patch as well.