Page MenuHomePhabricator

[RISCV] Define types for Zvlsseg.
Needs ReviewPublic

Authored by HsiangKai on Feb 23 2021, 1:42 AM.

Details

Summary

The types in Zvlsseg occupy multiple vector registers. We use Record type in clang to represent Zvlsseg types. It will not increase the number of builtin types in clang. It just adds multiple implicit defined Record types for Zvlsseg.

Diff Detail

Event Timeline

HsiangKai created this revision.Feb 23 2021, 1:42 AM
HsiangKai requested review of this revision.Feb 23 2021, 1:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 1:42 AM
craig.topper added inline comments.Feb 23 2021, 2:48 PM
clang/include/clang/Basic/RISCVVTypes.def
66–67

Do we still need NF on these macros if tuples have their own macros?

81

What is the last macro argument? This names it "false"

clang/lib/Sema/Sema.cpp
403

This also calls the last argument "false"

craig.topper added inline comments.Feb 23 2021, 2:55 PM
clang/lib/CodeGen/TargetInfo.cpp
10672 ↗(On Diff #325712)

Are we able to test this yet?

craig.topper added inline comments.Feb 23 2021, 3:00 PM
clang/include/clang/AST/ASTContext.h
1049

Why is this QualType and not CanQualType? I don't know what the difference is myself.

craig.topper added inline comments.Feb 23 2021, 3:53 PM
clang/include/clang/AST/ASTContext.h
1049

Is there any downside to adding all these members to the ASTContext? I suspect the builtin types have grown quite a bit since this was initially set up. @rjmccall or @rsmith is this something we need to be concerned about?

HsiangKai updated this revision to Diff 325957.Feb 23 2021, 6:49 PM
HsiangKai marked 3 inline comments as done.
HsiangKai added inline comments.
clang/include/clang/AST/ASTContext.h
1049

CanQualType could not be used for typedef types.

clang/lib/CodeGen/TargetInfo.cpp
10672 ↗(On Diff #325712)

No, I removed it.

craig.topper added inline comments.Feb 23 2021, 7:05 PM
clang/include/clang/AST/ASTContext.h
1049

Probabl you should write a comment explaining why since very line above this uses CanQualType.

Add comments.

HsiangKai marked an inline comment as done.Feb 23 2021, 10:26 PM
rogfer01 added a comment.EditedWed, Mar 31, 11:49 PM

I was under the impression we didn't want to use class-member access syntax for vector tuples (see https://github.com/riscv/rvv-intrinsic-doc/issues/17#issuecomment-628998077 ) so we don't need a record type, do we?

Perhaps it is possible to model them like opaque entities similar to what we do with RVV vectors where they are expanded in CodegenTypes.cpp?

Access to fields would have to be through intrinsics, though I think this didn't scale very well, did it?

rogfer01 added inline comments.Thu, Apr 1, 12:48 AM
clang/lib/AST/ASTContext.cpp
1486

I'm confused by this lookup.

If you add RvvInt8mf8Ty in the macros above will you be able to use it for types such as RvvInt8mf8x2Ty, RvvInt8mf8x3Ty, etc. without having to search it in all the types?

AFAICT ElemId seems only used in this case so perhaps we can use the "name of the field of ASTContext" (a CanQualType) directly.

I was under the impression we didn't want to use class-member access syntax for vector tuples (see https://github.com/riscv/rvv-intrinsic-doc/issues/17#issuecomment-628998077 ) so we don't need a record type, do we?

Perhaps it is possible to model them like opaque entities similar to what we do with RVV vectors where they are expanded in CodegenTypes.cpp?

Access to fields would have to be through intrinsics, though I think this didn't scale very well, did it?

Do you mean treat the Zvlsseg types as kind of builtin types? I want to avoid to add too much builtin types in Clang. That is why I use implicit defined RecordType for Zvlsseg types.

HsiangKai updated this revision to Diff 336320.Thu, Apr 8, 11:23 PM

Put element types in the macro and use them to create RecordType directly.

HsiangKai marked an inline comment as done.Thu, Apr 8, 11:23 PM