[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-02 Thread Florian Hahn via Phabricator via cfe-commits
fhahn marked an inline comment as done. fhahn added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:4738-4739 +bool ConversionOk = tryConvertToTy(*this, Context.getSizeType(), &ConvExpr); +assert(ConversionOk && + "should be able to convert any integer

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-01 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:4738-4739 +bool ConversionOk = tryConvertToTy(*this, Context.getSizeType(), &ConvExpr); +assert(ConversionOk && + "should be able to convert any integer type to size type"); +ret

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-01 Thread Florian Hahn via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8f3f88d2f50d: [Matrix] Implement matrix index expressions ([][]). (authored by fhahn). Changed prior to commit: https://reviews.llvm.org/D76791?vs=267601&id=267701#toc Repository: rG LLVM Github Mono

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-01 Thread Florian Hahn via Phabricator via cfe-commits
fhahn marked an inline comment as done. fhahn added a comment. Thanks for all the feedback John! Comment at: clang/lib/CodeGen/CGExpr.cpp:3787 + ColIdx->getType()->getScalarSizeInBits()); + llvm::Type *IntTy = llvm::IntegerType::get(getLLVMContext

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-01 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM, thanks. Comment at: clang/lib/CodeGen/CGExpr.cpp:3787 + ColIdx->getType()->getScalarSizeInBits()); + llvm::Type *IntTy = llvm::Intege

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-01 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 267601. fhahn added a comment. Force index types to size_t in Sema, remove code to extend index values in Codegen. Update tests to check more targeted IR. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76791/new/

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-06-01 Thread Florian Hahn via Phabricator via cfe-commits
fhahn marked 2 inline comments as done. fhahn added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:3787 + ColIdx->getType()->getScalarSizeInBits()); + llvm::Type *IntTy = llvm::IntegerType::get(getLLVMContext(), MaxWidth); + RowIdx = Bui

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-29 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D76791#2064434 , @fhahn wrote: > In D76791#2063926 , @rjmccall wrote: > > > > Yes at the moment I think we want to limit element wise > > > accesses/modifications to go through the acce

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-29 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment. In D76791#2063926 , @rjmccall wrote: > > Yes at the moment I think we want to limit element wise > > accesses/modifications to go through the access operator only, to guarantee > > we can rely on the vector forms in codegen. > > >

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-29 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. > Yes at the moment I think we want to limit element wise > accesses/modifications to go through the access operator only, to guarantee > we can rely on the vector forms in codegen. > > Additionally I think at least initially we want to avoid handing out pointers > to

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-29 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 267296. fhahn marked 5 inline comments as done. fhahn added a comment. Thanks for the latest round of comments! All expect one should be addressed. For the remaining comment, I responded inline. Also, it would be great if you could let me know if the updated c

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. I also want to confirm explicitly that you've decided that the right language design is for matrix components to not be addressable. You're convinced that that's an important restriction to get the code generation you want? Repository: rG LLVM Github Monorepo CHAN

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:1900 + assert(!LV.isMatrixElt() && + "loads of matrix element LValues should be handled elsewhere"); assert(LV.isBitField() && "Unknown LValue type!"); This should be handled her

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 266643. fhahn marked 4 inline comments as done. fhahn added a comment. Addressed latest comments: - Handle placeholder types in CreateBuiltinMatrixSubscriptExpr and do not limit to non-overload types there. - Check !MatrixSubscriptExpr instead of ParenExpr. -

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:4650 + (Base->isTypeDependent() || RowIdx->isTypeDependent() || + (ColumnIdx && ColumnIdx->isTypeDependent( +return new (Context) MatrixSubscriptExpr(Base, RowIdx, ColumnIdx,

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:4650 + (Base->isTypeDependent() || RowIdx->isTypeDependent() || + (ColumnIdx && ColumnIdx->isTypeDependent( +return new (Context) MatrixSubscriptExpr(Base, RowIdx, ColumnIdx, --

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added inline comments. Comment at: clang/include/clang/AST/Expr.h:2648 +/// MatrixSubscriptExpr - Matrix subscript expression for the MatrixType +/// extension. +class MatrixSubscriptExpr : public Expr { rjmccall wrote: > fhahn wrote: > > rjmccall wrote: >

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 266602. fhahn marked 10 inline comments as done. fhahn added a comment. Addressed latest round of comments, thanks! Changes include: - OK_MatrixComponent now behaves like OK_VectorComponent with respect to taking address/reference. - Look through non-overload

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/include/clang/AST/Expr.h:2648 +/// MatrixSubscriptExpr - Matrix subscript expression for the MatrixType +/// extension. +class MatrixSubscriptExpr : public Expr { fhahn wrote: > rjmccall wrote: > > Oh, that's inte

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-26 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 266205. fhahn marked 7 inline comments as done. fhahn added a comment. Address John's latest comments, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76791/new/ https://reviews.llvm.org/D76791 Files: cl

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-26 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added inline comments. Comment at: clang/include/clang/AST/Expr.h:2648 +/// MatrixSubscriptExpr - Matrix subscript expression for the MatrixType +/// extension. +class MatrixSubscriptExpr : public Expr { rjmccall wrote: > Oh, that's interesting. So you've

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-22 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments. Comment at: clang/include/clang/AST/Expr.h:2648 +/// MatrixSubscriptExpr - Matrix subscript expression for the MatrixType +/// extension. +class MatrixSubscriptExpr : public Expr { Oh, that's interesting. So you've changed this to

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-22 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 265750. fhahn added a comment. Add clarifying comment to ActOnMatrixSubscriptExpr calls in ActOnArraySubscriptExpr. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76791/new/ https://reviews.llvm.org/D76791 Files

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-22 Thread Florian Hahn via Phabricator via cfe-commits
fhahn marked an inline comment as done. fhahn added inline comments. Comment at: clang/lib/AST/Expr.cpp:3859 + + auto *SubscriptE = dyn_cast(this); + return SubscriptE rjmccall wrote: > You need to `IgnoreParens()` here. This code is now gone. ===

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-22 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 265728. fhahn marked 4 inline comments as done. fhahn added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. In D76791#2048387 , @rjmccall wrote: > Sorry for the slow review; I'm getting

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Sorry for the slow review; I'm getting crushed with review requests. Comment at: clang/lib/AST/Expr.cpp:3859 + + auto *SubscriptE = dyn_cast(this); + return SubscriptE You need to `IgnoreParens()` here. Comment at:

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-14 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 264094. fhahn added a comment. Update to support non-constant-integer-expressions as indices. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76791/new/ https://reviews.llvm.org/D76791 Files: clang/include/clang

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-05-08 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 262893. fhahn added a comment. Rebase on top of the latest version of D72281 . Refactor code to re-use new address conversion helper. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-04-01 Thread Florian Hahn via Phabricator via cfe-commits
fhahn updated this revision to Diff 254228. fhahn added a comment. Use placeholder type for incomplete matrix index expressions, as suggested by @rjmccall Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76791/new/ https://reviews.llvm.org/D76791 Fi

[PATCH] D76791: [Matrix] Implement matrix index expressions ([][]).

2020-03-25 Thread Florian Hahn via Phabricator via cfe-commits
fhahn created this revision. fhahn added reviewers: rjmccall, anemet, Bigcheese, rsmith, martong. Herald added subscribers: tschuett, arphaman, dexonsmith, rnkovacs. Herald added a project: clang. This patch implements matrix index expressions (matrix[RowIdx][ColumnIdx]). It does so by utilizing