-
Notifications
You must be signed in to change notification settings - Fork 14k
Enable matrices in HLSL #111415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Enable matrices in HLSL #111415
Changes from all commits
f4751fc
1fa442b
58d5186
3f53681
ae1a58b
eeb3165
75f19e4
0531fc9
fa26735
6ef5a91
de15228
fa5b05a
1e65231
767446a
d9ac6e7
cbf13a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -852,10 +852,18 @@ void TypePrinter::printExtVectorAfter(const ExtVectorType *T, raw_ostream &OS) { | |
|
||
void TypePrinter::printConstantMatrixBefore(const ConstantMatrixType *T, | ||
raw_ostream &OS) { | ||
if (Policy.UseHLSLTypes) | ||
OS << "matrix<"; | ||
printBefore(T->getElementType(), OS); | ||
OS << " __attribute__((matrix_type("; | ||
if (!Policy.UseHLSLTypes) | ||
OS << " __attribute__((matrix_type("; | ||
else | ||
OS << ", "; | ||
OS << T->getNumRows() << ", " << T->getNumColumns(); | ||
OS << ")))"; | ||
if (!Policy.UseHLSLTypes) | ||
OS << ")))"; | ||
else | ||
OS << ">"; | ||
} | ||
|
||
void TypePrinter::printConstantMatrixAfter(const ConstantMatrixType *T, | ||
|
@@ -865,16 +873,25 @@ void TypePrinter::printConstantMatrixAfter(const ConstantMatrixType *T, | |
|
||
void TypePrinter::printDependentSizedMatrixBefore( | ||
const DependentSizedMatrixType *T, raw_ostream &OS) { | ||
if (Policy.UseHLSLTypes) | ||
OS << "matrix<"; | ||
printBefore(T->getElementType(), OS); | ||
OS << " __attribute__((matrix_type("; | ||
if (T->getRowExpr()) { | ||
T->getRowExpr()->printPretty(OS, nullptr, Policy); | ||
} | ||
if (!Policy.UseHLSLTypes) | ||
OS << " __attribute__((matrix_type("; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You still have this in the "after" function. That will print this after pointer annotations which changes its meaning incorrectly. I'm guessing there isn't an existing test case for this, otherwise this change would have broken something. cc: @fhahn There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I guess I missed this one. AFAICT, DependentSizedMatrices are never printed. |
||
else | ||
OS << ", "; | ||
|
||
if (Expr *E = T->getRowExpr()) | ||
E->printPretty(OS, nullptr, Policy); | ||
OS << ", "; | ||
if (T->getColumnExpr()) { | ||
T->getColumnExpr()->printPretty(OS, nullptr, Policy); | ||
} | ||
OS << ")))"; | ||
if (Expr *E = T->getColumnExpr()) | ||
E->printPretty(OS, nullptr, Policy); | ||
|
||
OS << ", "; | ||
if (!Policy.UseHLSLTypes) | ||
OS << ")))"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The changes to this file do alter how pointer and reference matrices are printed. I'd like to get @fhahn's opinion on this even if on nothing else. |
||
else | ||
OS << ">"; | ||
} | ||
|
||
void TypePrinter::printDependentSizedMatrixAfter( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -459,8 +459,81 @@ void HLSLExternalSemaSource::defineHLSLVectorAlias() { | |
HLSLNamespace->addDecl(Template); | ||
} | ||
|
||
void HLSLExternalSemaSource::defineHLSLMatrixAlias() { | ||
ASTContext &AST = SemaPtr->getASTContext(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment here explaining what alias is added? |
||
|
||
llvm::SmallVector<NamedDecl *> TemplateParams; | ||
|
||
auto *TypeParam = TemplateTypeParmDecl::Create( | ||
AST, HLSLNamespace, SourceLocation(), SourceLocation(), 0, 0, | ||
&AST.Idents.get("element", tok::TokenKind::identifier), false, false); | ||
TypeParam->setDefaultArgument( | ||
AST, SemaPtr->getTrivialTemplateArgumentLoc( | ||
TemplateArgument(AST.FloatTy), QualType(), SourceLocation())); | ||
|
||
TemplateParams.emplace_back(TypeParam); | ||
|
||
// these should be 64 bit to be consistent with other clang matrices. | ||
auto *RowsParam = NonTypeTemplateParmDecl::Create( | ||
AST, HLSLNamespace, SourceLocation(), SourceLocation(), 0, 1, | ||
&AST.Idents.get("rows_count", tok::TokenKind::identifier), AST.IntTy, | ||
false, AST.getTrivialTypeSourceInfo(AST.IntTy)); | ||
llvm::APInt RVal(AST.getIntWidth(AST.IntTy), 4); | ||
TemplateArgument RDefault(AST, llvm::APSInt(std::move(RVal)), AST.IntTy, | ||
/*IsDefaulted=*/true); | ||
RowsParam->setDefaultArgument( | ||
AST, SemaPtr->getTrivialTemplateArgumentLoc(RDefault, AST.IntTy, | ||
SourceLocation(), RowsParam)); | ||
TemplateParams.emplace_back(RowsParam); | ||
|
||
auto *ColsParam = NonTypeTemplateParmDecl::Create( | ||
AST, HLSLNamespace, SourceLocation(), SourceLocation(), 0, 2, | ||
&AST.Idents.get("cols_count", tok::TokenKind::identifier), AST.IntTy, | ||
false, AST.getTrivialTypeSourceInfo(AST.IntTy)); | ||
llvm::APInt CVal(AST.getIntWidth(AST.IntTy), 4); | ||
TemplateArgument CDefault(AST, llvm::APSInt(std::move(CVal)), AST.IntTy, | ||
/*IsDefaulted=*/true); | ||
ColsParam->setDefaultArgument( | ||
AST, SemaPtr->getTrivialTemplateArgumentLoc(CDefault, AST.IntTy, | ||
SourceLocation(), ColsParam)); | ||
TemplateParams.emplace_back(RowsParam); | ||
|
||
auto *ParamList = | ||
TemplateParameterList::Create(AST, SourceLocation(), SourceLocation(), | ||
TemplateParams, SourceLocation(), nullptr); | ||
|
||
IdentifierInfo &II = AST.Idents.get("matrix", tok::TokenKind::identifier); | ||
|
||
QualType AliasType = AST.getDependentSizedMatrixType( | ||
AST.getTemplateTypeParmType(0, 0, false, TypeParam), | ||
DeclRefExpr::Create( | ||
AST, NestedNameSpecifierLoc(), SourceLocation(), RowsParam, false, | ||
DeclarationNameInfo(RowsParam->getDeclName(), SourceLocation()), | ||
AST.IntTy, VK_LValue), | ||
DeclRefExpr::Create( | ||
AST, NestedNameSpecifierLoc(), SourceLocation(), ColsParam, false, | ||
DeclarationNameInfo(ColsParam->getDeclName(), SourceLocation()), | ||
AST.IntTy, VK_LValue), | ||
SourceLocation()); | ||
|
||
auto *Record = TypeAliasDecl::Create(AST, HLSLNamespace, SourceLocation(), | ||
SourceLocation(), &II, | ||
AST.getTrivialTypeSourceInfo(AliasType)); | ||
Record->setImplicit(true); | ||
|
||
auto *Template = | ||
TypeAliasTemplateDecl::Create(AST, HLSLNamespace, SourceLocation(), | ||
Record->getIdentifier(), ParamList, Record); | ||
|
||
Record->setDescribedAliasTemplate(Template); | ||
Template->setImplicit(true); | ||
Template->setLexicalDeclContext(Record->getDeclContext()); | ||
HLSLNamespace->addDecl(Template); | ||
} | ||
|
||
void HLSLExternalSemaSource::defineTrivialHLSLTypes() { | ||
defineHLSLVectorAlias(); | ||
defineHLSLMatrixAlias(); | ||
} | ||
|
||
/// Set up common members and attributes for buffer types | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems wrong to me. IIUC this changes something like
float __attribute__((matrix_type(4, 4)))*
tofloat * __attribute__((matrix_type(4, 4)))
, which would mean the element type isfloat*
rather thanfloat
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my response to Florian here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be simpler to read to duplicate some code but have the HSL and C++ matrixes types printed completely separately?